[FIX]Windows not sized correctly by window.open

RESOLVED FIXED in mozilla1.9alpha1

Status

()

Core
Embedding: APIs
P1
normal
RESOLVED FIXED
12 years ago
9 years ago

People

(Reporter: bz, Assigned: bz)

Tracking

(Depends on: 1 bug, {regression})

Trunk
mozilla1.9alpha1
x86
Linux
regression
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

(URL)

Attachments

(1 attachment)

The testcase is in the URL field -- the window that opens should have a square content area.  Over here it ends up about twice as tall as it is wide.

This regressed between 2006-05-10-08 and 2006-05-11-05, so this is fallout from threadmanager somehow...  Not sure how.

Darin, let me know if you don't have time to look into this, ok?

Comment 1

12 years ago
Boris, this is on my list of threadmanager regressions that I'm gradually working to resolve.  Any help tracking it down would of course be appreciated ;-)
Created attachment 229923 [details] [diff] [review]
OK, I think this is the right fix

So what was happening was that when we called GetSize() on the newly opened content docshell in nsWindowWatcher::SizeOpenedDocShellItem we got bogus answers.  The reason they were bogus is that the restyle event posted when we changed the chromehidden attribute on the <window> hadn't been processed yet.  So our calculation of the size of the chrome was off by the difference between the default chrome and the chrome that actually appeared in the window (in my case by the height of the menubar and all the toolbars) -- they're all visible by default but hidden with the given window features.  Since window sizing is done on the outer window, not the content docshell, we have to take the given size and add the chrome size to get the thing to actually size to, so we were ending up too tall (by 140px, in my case, since that's the height of menu + toolbars).

The reason there is no restyle event processing happening is that we get the nsWebShellWindow::OnStateChange that sets mLockedUntilChromeLoad to PR_FALSE, and that calls nsXULWindow::OnChromeLoaded which actually messes with the chromehidden attribute and posts the restyle event.  Then we go back to the loop in nsXULWindow::CreateNewContentWindow and break out, since IsLocked() is now false.  I have no idea why this didn't happen with the old "spin up an appshell" approach, and I'm not sure it matters -- we should be doing the flush in docshell no matter what, imo.
Assignee: nobody → bzbarsky
Status: NEW → ASSIGNED
Attachment #229923 - Flags: superreview?(jst)
Attachment #229923 - Flags: review?(jst)
Priority: -- → P1
Summary: Windows not sized correctly by window.open → [FIX]Windows not sized correctly by window.open
Target Milestone: --- → mozilla1.9alpha
Comment on attachment 229923 [details] [diff] [review]
OK, I think this is the right fix

r+sr=jst
Attachment #229923 - Flags: superreview?(jst)
Attachment #229923 - Flags: superreview+
Attachment #229923 - Flags: review?(jst)
Attachment #229923 - Flags: review+
Fixed.
Status: ASSIGNED → RESOLVED
Last Resolved: 12 years ago
Resolution: --- → FIXED
Flags: in-testsuite?

Comment 5

11 years ago
in mochitest with a todo() call. See bug 358303.

RCS file: /cvsroot/mozilla/testing/mochitest/tests/test_bug344861.html,v
done
Checking in tests/test_bug344861.html;
/cvsroot/mozilla/testing/mochitest/tests/test_bug344861.html,v  <--  test_bug344861.html
initial revision: 1.1
done
Flags: in-testsuite? → in-testsuite+
Depends on: 345857
Depends on: 442682
You need to log in before you can comment on or make changes to this bug.