This is a general issue with ECMAScript bindings in Gecko. Unless the argument is explicitly marked as "optional", it's required. I believe this is correct behavior per existing specs (though so is the IE/Safari/Opera one). It's not clear to me what behavior WebIDL specifies here; that part of it was in flux last I checked. Current early drafts of CSSOM propose having a one-argument overload for getComputedStyle; once that stabilizes somewhat we would probably implement it. It, too, is in flux enough that wasting time implementing it right now only to have to rewrite in the near future isn't worth it.
Boris, do you realize that this currently forces all web developers to write `window.getComputedStyle(element, null)` *just* for the Firefox? Every other browser takes one argument just fine. Is it so hard to fix, really? I am perplexed by this. P.S. Sorry if I sound too angry/grumbling.
Somebody with canconfirm please change the status of this bug to NEW. Thanks.
I wholeheartedly agree with Sergei. I know of no other DOM methods that require you pass *null* for the final argument and I think it is just plain common sense to make it optional. It would be ideal for this issue to be fixed in Firefox 4.
Created attachment 480188 [details] [diff] [review] Make getComputedStyle's second argument optional.
Comment on attachment 480188 [details] [diff] [review] Make getComputedStyle's second argument optional. > [scriptable, uuid(0b9341f3-95d4-4fa4-adcd-e119e0db2889)] > interface nsIDOMViewCSS : nsIDOMAbstractView ... >- in DOMString pseudoElt); >+ [optional] in DOMString pseudoElt); Update the IID? That's the only thing I see off here.
Comment on attachment 480188 [details] [diff] [review] Make getComputedStyle's second argument optional. r=me. Please make sure to NOT change the iid here when landing, no matter what Alex says. This is a binary-compatible change, and a backwards-compatible change on the JS end, so does not need an iid change. Which is good, because this couldn't land for 2.0 at this stage if it needed to change the iid. ;)
7 years ago
Created attachment 483287 [details] [diff] [review] Patch for checkin
patching file content/base/test/Makefile.in Hunk #1 FAILED at 412 1 out of 1 hunks FAILED -- saving rejects to file content/base/test/Makefile.in.rej
Created attachment 483503 [details] [diff] [review] Patch for checkin Fixed, thanks.
Should this land on 1.9.2? Firefox 3.6 will live on a bit after 4.0 has been released, but web developers won't necessarily test their stuff there...
I'd be fine with doing that, I think.
Comment on attachment 483503 [details] [diff] [review] Patch for checkin Approved for 18.104.22.168, a=dveditz for release-drivers
It would probably be worth it taking it all the way back to 1.9.1 before we cut it loose, correct? Would that not be worth the risk? This looks to be virtually no risk with only benefit.
I backed this out as it had busted all of the 1.9.2 builds and there wasn't any sign of anyone to ask on irc, nor any indication of a clobber being required: http://hg.mozilla.org/releases/mozilla-1.9.2/rev/5b942ca87e90 Compiler errors: http://tinderbox.mozilla.org/showlog.cgi?log=Firefox3.6/1288172723.1288172904.2891.gz#err0 In file included from /builds/slave/mozilla-1.9.2-linux/build/js/src/nanojit/nanojit.h:267, from /builds/slave/mozilla-1.9.2-linux/build/js/src/jsbuiltins.h:45, from dom_quickstubs.cpp:12: /builds/slave/mozilla-1.9.2-linux/build/js/src/nanojit/Native.h:53:27: warning: anonymous variadic macros were introduced in C99 /builds/slave/mozilla-1.9.2-linux/build/js/src/nanojit/Native.h:54:23: warning: anonymous variadic macros were introduced in C99 /builds/slave/mozilla-1.9.2-linux/build/js/src/nanojit/Native.h:135:28: warning: anonymous variadic macros were introduced in C99 /builds/slave/mozilla-1.9.2-linux/build/js/src/jsfun.h: In function 'js_ArgsPrivateNative* js_GetArgsPrivateNative(JSObject*)': /builds/slave/mozilla-1.9.2-linux/build/js/src/jsfun.h:233: warning: converting to non-pointer type 'unsigned int' from NULL dom_quickstubs.cpp: In function 'JSBool nsIDOMViewCSS_GetComputedStyle(JSContext*, uintN, jsval*)': dom_quickstubs.cpp:7559: error: expected primary-expression before ')' token make: *** [dom_quickstubs.o] Error 1 make: Leaving directory `/builds/slave/mozilla-1.9.2-linux/build/obj-firefox/js/src/xpconnect/src'
I guess qsgen.py needs an update to for that fix. qsgen.py does not support optional ptr arguments. -> http://mxr.mozilla.org/mozilla1.9.2/source/js/src/xpconnect/src/qsgen.py#436
Documentation updated: https://developer.mozilla.org/en/DOM/window.getComputedStyle Updated Firefox 4 for developers as well.
Comment on attachment 483503 [details] [diff] [review] Patch for checkin un-approving this patch for 22.214.171.124 -- we'll need one that rolls in the bustage fix.
FTR, I'm not planning to get 1.9.2 set up, if someone feels strongly that this should be on branch, please feel free to write a patch.