[FIX] window.innerWidth throws exception when called on a non-displayed frame

RESOLVED FIXED in mozilla1.9beta2

Status

()

P3
normal
RESOLVED FIXED
11 years ago
11 years ago

People

(Reporter: ted, Assigned: jst)

Tracking

({regression})

Trunk
mozilla1.9beta2
x86
Windows XP
regression
Points:
---
Bug Flags:
blocking1.9 +
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

(URL)

Attachments

(3 attachments)

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.
(Assignee)

Comment 1

11 years ago
This is a trivial fix, we shouldn't ship with this regression IMO.
Status: NEW → ASSIGNED
Flags: blocking1.9?
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)
(Assignee)

Comment 4

11 years ago
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.
(Assignee)

Comment 6

11 years ago
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 #285289 - Flags: superreview?(jonas)
Attachment #285289 - Flags: review?(jonas)
(Assignee)

Updated

11 years ago
Attachment #285284 - Flags: review?(jst) → review+
Attachment #285289 - Flags: superreview?(jonas)
Attachment #285289 - Flags: superreview+
Attachment #285289 - Flags: review?(jonas)
Attachment #285289 - Flags: 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?
(Assignee)

Comment 8

11 years ago
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.
(Assignee)

Updated

11 years ago
Target Milestone: --- → mozilla1.9 M10
(Assignee)

Updated

11 years ago
Flags: blocking1.9? → blocking1.9+
(Assignee)

Updated

11 years ago
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
(Assignee)

Comment 10

11 years ago
Fix checked in.
Status: ASSIGNED → RESOLVED
Last Resolved: 11 years ago
Resolution: --- → FIXED
Neglected to check in that mochitest, just did it now.
Flags: in-testsuite+
(Assignee)

Comment 12

11 years ago
Duh, thanks Ted!
You need to log in before you can comment on or make changes to this bug.