Support min / max size for top level windows that have <html> root element
Categories
(Core :: Window Management, task, P3)
Tracking
()
Tracking | Status | |
---|---|---|
firefox71 | --- | fixed |
People
(Reporter: bgrins, Assigned: bdahl)
References
Details
Attachments
(2 files)
Some background: https://bugzilla.mozilla.org/show_bug.cgi?id=1492582#c35 and https://bugzilla.mozilla.org/show_bug.cgi?id=1492582#c45.
In Bug 1492582 while attempting to migrate from <xul:window> to <html> in browser.xhtml, I noticed that the window can get resized much smaller than before. This is due to nsContainerFrame::SyncWindowProperties
assuming we have a xul sized root element.
Reporter | ||
Comment 1•6 years ago
|
||
From https://bugzilla.mozilla.org/show_bug.cgi?id=1492582#c54 - potential fix for the min / max-width issue:
diff --git a/layout/generic/nsContainerFrame.cpp b/layout/generic/nsContainerFrame.cpp
index c1616f985a93..ae810eb57b88 100644
--- a/layout/generic/nsContainerFrame.cpp
+++ b/layout/generic/nsContainerFrame.cpp
@@ -612,10 +612,21 @@ void nsContainerFrame::SyncWindowProperties(nsPresContext* aPresContext,
return;
}
- nsBoxLayoutState aState(aPresContext, aRC);
- nsSize minSize = rootFrame->GetXULMinSize(aState);
- nsSize maxSize = rootFrame->GetXULMaxSize(aState);
-
+ auto* pos = rootFrame->StylePosition();
+ nsSize minSize(0, 0);
+ nsSize maxSize(NS_UNCONSTRAINEDSIZE, NS_UNCONSTRAINEDSIZE);
+ if (pos->mMinWidth.ConvertsToLength()) {
+ minSize.width = pos->mMinWidth.ToLength();
+ }
+ if (pos->mMinHeight.ConvertsToLength()) {
+ minSize.height = pos->mMinHeight.ToLength();
+ }
+ if (pos->mMaxWidth.ConvertsToLength()) {
+ maxSize.width = pos->mMaxWidth.ToLength();
+ }
+ if (pos->mMaxHeight.ConvertsToLength()) {
+ maxSize.height = pos->mMaxHeight.ToLength();
+ }
SetSizeConstraints(aPresContext, windowWidget, minSize, maxSize);
#endif
}
Reporter | ||
Comment 2•6 years ago
|
||
The fix in Comment 1 generally works with only a couple of test failures: https://treeherder.mozilla.org/#/jobs?repo=try&revision=6a7df52d922a2e74d7239ebe78f06648da9a66a4.
When I applied it in Bug 1492582 I noticed that I also had to explicitly set min-width in CSS. I think this may be due to the cause of one of the test failures above (test_window_intrinsic_size.xul) which is that the xul sizing seems to take some kind of implicit size into account based on the children nodes, while that fix requires an explicit size to be specified.
It might be OK to require an explicit size since we have so few top level windows. Although if it's easy to take the same thing into account for HTML sizing that would be more of a drop-in change (I'm not sure if we have untested code relying on this behavior).
Updated•6 years ago
|
Assignee | ||
Comment 3•6 years ago
|
||
Neil,
Would you be okay with the change above where we'd require a min-width/height be set instead of using GetXULMinSize?
For more context from IRC:
<bdahl>: emilio: is there a way to compute an html elements intrinsic size, similar to XUL's GetXULMinSize? we could use your patch but that requires setting an explicit min-width
or i guess if we're okay setting an explicit min-width/height that's fine too
<emilio> bdahl: not without doing layout... If you're guaranteed to have clean layout you can call GetMinISize
1:17 PM bdahl: not sure how to most easily get a proper rendering context out of the blue though
Assignee | ||
Comment 5•6 years ago
|
||
Helps support migrating from XUL DOM to an HTML DOM structure. I attempted
to remove the early return when the root element is not a XUL element, but
it appears setting the transparency on a non XUL root is still broken
(test widget/tests/test_mouse_scroll.xul started failing on windows).
Comment 7•6 years ago
|
||
bugherder |
Comment 8•6 years ago
|
||
Backed out for perma fails on browser_startup_syncIPC.js, unexpected PLayerTransaction::Msg_GetTextureFactoryIdentifier sync IPC before first paint.
Failure log: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=260643664&repo=autoland&lineNumber=1572
[task 2019-08-08T19:10:10.933Z] 19:10:10 INFO - TEST-START | browser/base/content/test/performance/browser_startup_mainthreadio.js
[task 2019-08-08T19:10:11.157Z] 19:10:11 INFO - GECKO(6956) | MEMORY STAT | vsize 2104173MB | vsizeMaxContiguous 66645103MB | residentFast 283MB | heapAllocated 138MB
[task 2019-08-08T19:10:11.157Z] 19:10:11 INFO - TEST-OK | browser/base/content/test/performance/browser_startup_mainthreadio.js | took 222ms
[task 2019-08-08T19:10:11.187Z] 19:10:11 INFO - checking window state
[task 2019-08-08T19:10:11.187Z] 19:10:11 INFO - GECKO(6956) | JavaScript error: resource://testing-common/PromiseTestUtils.jsm, line 112: uncaught exception: Object
[task 2019-08-08T19:10:11.188Z] 19:10:11 INFO - TEST-START | browser/base/content/test/performance/browser_startup_syncIPC.js
[task 2019-08-08T19:10:11.198Z] 19:10:11 INFO - TEST-INFO | started process screenshot
[task 2019-08-08T19:10:11.277Z] 19:10:11 INFO - TEST-INFO | screenshot: exit 0
[task 2019-08-08T19:10:11.277Z] 19:10:11 INFO - Buffered messages logged at 19:10:11
[task 2019-08-08T19:10:11.278Z] 19:10:11 INFO - Entering test bound
[task 2019-08-08T19:10:11.279Z] 19:10:11 INFO - whitelisted sync IPC before first paint:
[task 2019-08-08T19:10:11.279Z] 19:10:11 INFO - PWebRenderBridge::Msg_EnsureConnected - at most 2 times
[task 2019-08-08T19:10:11.280Z] 19:10:11 INFO - PCompositorBridge::Msg_NotifyChildCreated - at most 2 times
[task 2019-08-08T19:10:11.280Z] 19:10:11 INFO - PCompositorBridge::Msg_MapAndNotifyChildCreated - at most 2 times
[task 2019-08-08T19:10:11.281Z] 19:10:11 INFO - PCompositorBridge::Msg_FlushRendering - at most 1 times
[task 2019-08-08T19:10:11.281Z] 19:10:11 INFO - PCompositorBridge::Msg_Initialize - at most 3 times
[task 2019-08-08T19:10:11.281Z] 19:10:11 INFO - PGPU::Msg_AddLayerTreeIdMapping - at most 5 times
[task 2019-08-08T19:10:11.282Z] 19:10:11 INFO - PCompositorBridge::Msg_WillClose - at most 2 times
[task 2019-08-08T19:10:11.282Z] 19:10:11 INFO - PAPZInputBridge::Msg_ProcessUnhandledEvent - at most 1 times
[task 2019-08-08T19:10:11.282Z] 19:10:11 INFO - PGPU::Msg_GetDeviceStatus - at most 1 times
[task 2019-08-08T19:10:11.283Z] 19:10:11 INFO - Buffered messages finished
[task 2019-08-08T19:10:11.283Z] 19:10:11 INFO - TEST-UNEXPECTED-FAIL | browser/base/content/test/performance/browser_startup_syncIPC.js | unexpected PLayerTransaction::Msg_GetTextureFactoryIdentifier sync IPC before first paint -
[task 2019-08-08T19:10:11.284Z] 19:10:11 INFO - Stack trace:
[task 2019-08-08T19:10:11.284Z] 19:10:11 INFO - chrome://mochikit/content/browser-test.js:test_ok:1576
[task 2019-08-08T19:10:11.284Z] 19:10:11 INFO - chrome://mochitests/content/browser/browser/base/content/test/performance/browser_startup_syncIPC.js:null:317
[task 2019-08-08T19:10:11.284Z] 19:10:11 INFO - chrome://mochikit/content/browser-test.js:Tester_execTest/<:1346
[task 2019-08-08T19:10:11.285Z] 19:10:11 INFO - chrome://mochikit/content/browser-test.js:Tester_execTest:1381
[task 2019-08-08T19:10:11.285Z] 19:10:11 INFO - chrome://mochikit/content/browser-test.js:nextTest/<:1209
[task 2019-08-08T19:10:11.285Z] 19:10:11 INFO - chrome://mochikit/content/tests/SimpleTest/SimpleTest.js:SimpleTest.waitForFocus/waitForFocusInner/focusedOrLoaded/<:803
Backout: https://hg.mozilla.org/mozilla-central/rev/dfac7f4ddfae815ea9077956ce3152bd36944bf6
Updated•6 years ago
|
Assignee | ||
Comment 9•5 years ago
|
||
Avoids IPC to get the max texture size until the layer manager is created.
Depends on D40734
Assignee | ||
Updated•5 years ago
|
Comment 10•5 years ago
|
||
Comment 11•5 years ago
|
||
bugherder |
Comment 12•5 years ago
|
||
Comment 13•5 years ago
|
||
bugherder |
Description
•