Closed Bug 377539 Opened 16 years ago Closed 16 years ago

nsIDOMScreen.width exception with display:none in parent frame

Categories

(Core :: DOM: Core & HTML, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla1.9alpha5

People

(Reporter: raccettura, Assigned: MatsPalmgren_bugz)

References

Details

(Keywords: testcase)

Attachments

(5 files, 1 obsolete file)

When getting the screen.width from within an iframe that is display:none, nsIDOMScreenwidth fails.  

Considering if display:none weren't in place it would return the size of the parent... I think it should do so regardless of the style.

[Exception... "Component returned failure code: 0x80004005 (NS_ERROR_FAILURE) [nsIDOMScreen.width]" nsresult: "0x80004005 (NS_ERROR_FAILURE)" location: "JS frame ::

Testcase coming.
Attached file Testcase (Frame)
Attached file Testcase (Page)
Clicking on the word "test" in attachment 261582 [details] shall serve as a testcase.
Testing this in IE shows that my assumption of it still returning the parent size was correct.
The testcase works in Opera 9.02 too.
Keywords: testcase
Attached patch wip (diff -w) (obsolete) — Splinter Review
The pres context is null.
http://bonsai.mozilla.org/cvsblame.cgi?file=/mozilla/dom/src/base/nsScreen.cpp&rev=1.28&root=/cvsroot&mark=199,227#199

Maybe we should look for one in an ancestor docshell?
What happens in a multi-monitor setup?  Note that there, the screen for a subframe may not be the same as the screen for the parent...

That said, if we have no display, using the screen of the parent makes sense.  So in general this seems reasonable.
(In reply to comment #7)
> What happens in a multi-monitor setup?  Note that there, the screen for a
> subframe may not be the same as the screen for the parent...

Ok, but it can't be completely window-less (native) and still be
associated with a different screen can it?
I can run a test if you can suggest how I would create the case you're
looking for.
An alternative solution would be to get the device context through
  docshell.QI(nsIBaseWindow)->GetMainWidget()->GetDeviceContext()
instead of what we do now
  docshell->GetContentViewer()->GetPresContext()->DeviceContext()

With the alternative solution we find the device context on the first
docshell for the testcase.
> Ok, but it can't be completely window-less (native) and still be
> associated with a different screen can it?

Right.  That's what the second half of comment 7 was about...
Assignee: general → mats.palmgren
Attachment #261629 - Attachment description: wip2 (diff -w) → Patch rev. 1 (diff -w)
Attachment #261629 - Flags: superreview?(bzbarsky)
Attachment #261629 - Flags: review?(bzbarsky)
Attached patch Patch rev. 1Splinter Review
Attachment #261620 - Attachment is obsolete: true
Comment on attachment 261629 [details] [diff] [review]
Patch rev. 1 (diff -w)

Yeah, looks reasonable.
Attachment #261629 - Flags: superreview?(bzbarsky)
Attachment #261629 - Flags: superreview+
Attachment #261629 - Flags: review?(bzbarsky)
Attachment #261629 - Flags: review+
Checked in to trunk at 2007-05-08 02:44 PDT.

-> FIXED
Status: NEW → RESOLVED
Closed: 16 years ago
Flags: in-testsuite?
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9alpha5
Add mochitest dom/tests/mochitest/bugs/test_bug377539-1.html
Attachment #264093 - Flags: review?(bzbarsky)
Comment on attachment 264093 [details] [diff] [review]
Mochitest, rev. 1

>Index: dom/tests/mochitest/bugs/Makefile.in
>+		test_bug377539-1.html \

Do you expect more mochitests for this bug?  If not, why bother with the "-1"?

>+<div id="content" style="display: none">

Why not just put the iframe in here instead of the dynamic stuff you do?  I think it would be clearer...

The rest looks good.
Attachment #264093 - Flags: review?(bzbarsky) → review+
I fixed the filename and checked in the mochitest at 2007-05-09 06:27 PDT.
I tried to remove the dynamic stuff but it didn't work, I don't know why,
my JS sucks, feel free to fix it ;-)
Flags: in-testsuite? → in-testsuite+
Blocks: 325180
Depends on: 405479
Depends on: 445765
You need to log in before you can comment on or make changes to this bug.