The testsuite in the URL apparently tries to access innerWidth on an iframe that isn't displayed. This works in FF2, but now throws an exception in trunk builds. STR: 1) Load the URL 2) Click the second run button in the list on the left (first one times out from a remote host...) 3) Test does not load, exception in Error Console: Error: uncaught exception: [Exception... "Component returned failure code: 0x80004005 (NS_ERROR_FAILURE) [nsIDOMWindowInternal.innerWidth]" nsresult: "0x80004005 (NS_ERROR_FAILURE)" location: "JS frame :: http://mavra.perilith.com/~luser/openlayers/tests/run-tests.html :: anonymous :: line 902" data: no] Should be easy enough to write an automated test for.
This is a trivial fix, we shouldn't ship with this regression IMO.
Status: NEW → ASSIGNED
Created attachment 285283 [details] simple testcase Will show "FAIL" if it gets an exception, otherwise will print the innerWidth/Height twice and then PASS. Tries to access innerWidth/Height, then set both of them, then access them again. I'll convert this to a mochitest as well.
Created attachment 285284 [details] [diff] [review] mochitest mochitest, in patch form. This fails 4/6 on trunk (interestingly enough setting the values passes), and passes 6/6 on latest-1.8.
Attachment #285284 - Flags: review?(jst)
Interesting. I bet setting the value succeeds because it's set on a frame, and setting the innerWidht/Height on a frame is a no-op. Given that, I think would be safe to leave the setters alone for the [i]frame case, but I'll still fix them as well to match how we used to deal with this in Firefox 2 when dealing with a closed window or what not.
I could add more tests if you'd like to test closed windows or something, just give me a pointer as to what we'd be testing.
Created attachment 285289 [details] [diff] [review] Fix. Don't throw exceptions, just return 0. This fixes both the getters and setters. I don't actually know if the setter change is testable, as closing a window will null out its mDocShell, so we'd never get to the piece of code that's changing here. I'd still prefere to make that change though for consistency with the getters if nothing else.
Attachment #285284 - Flags: review?(jst) → review+
Comment on attachment 285289 [details] [diff] [review] Fix. Don't throw exceptions, just return 0. Maybe I'm missing something, but does this patch actually init the out params to 0 if presContext is NULL?
The code that's changed here already does that, but it's earlier in the code than what you can see in the context in the diff.
Ah, bugzilla just doesn't play nice with your git diffs.
Summary: window.innerWidth throws exception when called on a non-displayed frame → [FIX] window.innerWidth throws exception when called on a non-displayed frame
Priority: -- → P3
Fix checked in.
Status: ASSIGNED → RESOLVED
Last Resolved: 11 years ago
Resolution: --- → FIXED
Neglected to check in that mochitest, just did it now.
Duh, thanks Ted!
You need to log in before you can comment on or make changes to this bug.