r103560 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:r103559‎ | r103560 | r103561 >
Date:02:14, 18 November 2011
Author:dale
Status:ok (Comments)
Tags:
Comment:
removed iframe ref in java getPlayerElement
Modified paths:
  • /trunk/extensions/TimedMediaHandler/MwEmbedModules/EmbedPlayer/resources/mw.EmbedPlayerJava.js (modified) (history)

Diff [purge]

Index: trunk/extensions/TimedMediaHandler/MwEmbedModules/EmbedPlayer/resources/mw.EmbedPlayerJava.js
@@ -73,9 +73,10 @@
7474 var mediaSrc = this.getSrc();
7575 var appletLoc = false;
7676 if (
77 - !mw.isLocalDomain( mediaSrc )
78 - ||
79 - !mw.isLocalDomain( mw.getMwEmbedPath() )
 77+ !( mw.isLocalDomain( mediaSrc )
 78+ ||
 79+ mw.isLocalDomain( mw.getMwEmbedPath() )
 80+ )
8081 ){
8182 if ( window.cortadoDomainLocations[ new mw.Uri( mediaSrc ).host ] ) {
8283 appletLoc = window.cortadoDomainLocations[ new mw.Uri( mediaSrc ).host ];
@@ -169,13 +170,6 @@
170171 }
171172 //mw.log( 'getPlayerElement::' + this.pid );
172173 this.playerElement = $( '#' + this.pid ).get( 0 );
173 - //this.playerElement = document.applets[ 0 ];
174 - // NOTE we are currently not using the iframe embed method:
175 - //if ( $.browser.mozilla ) {
176 - // this.playerElement = $('#cframe_' + this.id).contents().find( '#' + this.pid );
177 - //} else {
178 - // this.playerElement = $( '#' + this.pid ).get( 0 );
179 - //}
180174 return this.playerElement;
181175 },
182176

Follow-up revisions

RevisionCommit summaryAuthorDate
r103870followup r103560 -- hopefully this is the logic wanted: if neither mediaSrc n...neilk23:42, 21 November 2011

Comments

#Comment by Raindrift (talk | contribs)   01:34, 19 November 2011
mw.isLocalDomain( mw.getMwEmbedPath() )

Did you mean to not negate that any longer? I only ask because it's not mentioned in the commit message, seems like it could be a typo, or intentional.

#Comment by Mdale (talk | contribs)   02:10, 19 November 2011

( !a || !b ) === !( a || b ) .. changed per ( your ? ) request: http://www.mediawiki.org/wiki/TimedMediaHandler/ReviewNotes

#Comment by Raindrift (talk | contribs)   02:24, 19 November 2011

I don't always herp, but when I do, I derp.

(ok'd)

#Comment by Catrope (talk | contribs)   11:15, 19 November 2011

This is wrong. ( !a || !b) === !( a && b ) , not the same as !( a || b )

w:De Morgan's Law

#Comment by Mdale (talk | contribs)   19:26, 19 November 2011

happy to leave it as it was.. only changed on request.

#Comment by NeilK (talk | contribs)   23:46, 21 November 2011

If you were copying my notes (it was probably me because I'm persnickety about simplifying logic) I said "consider !(a && b)" on the wiki page.

Not a priority, just a suggestion. Anyway, I did it myself in r103870.

#Comment by Mdale (talk | contribs)   01:04, 22 November 2011

oh right, my bad sorry for the extra bug traffic

Status & tagging log