Closed Bug 1561326 Opened 5 years ago Closed 5 years ago

Support min / max size for top level windows that have <html> root element

Tracking

()

Status:

RESOLVED FIXED

Milestone:

mozilla71

Tracking Flags:

Tracking

Status

firefox71

---

fixed

People

(Reporter: bgrins, Assigned: bdahl)

References

Details

Attachments

(2 files)

Bug 1561326 - Support CSS min/max width/height for top level windows with root HTML element. r=emilio 5 years ago Brendan Dahl [:bdahl] 47 bytes, text/x-phabricator-request		Details \| Review
Bug 1561326 - Lazily constrain window size on Windows. r=mattwoodrow 5 years ago Brendan Dahl [:bdahl] 47 bytes, text/x-phabricator-request		Details \| Review

Brian Grinstead [:bgrins]

Reporter

Description

•

5 years ago

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.

Brian Grinstead [:bgrins]

Reporter

Comment 1

•

5 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
 }

Brian Grinstead [:bgrins]

Reporter

Comment 2

•

5 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).

Neil Deakin

Updated

•

5 years ago

Priority: -- → P3

Brendan Dahl [:bdahl]

Assignee

Comment 3

•

5 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

Flags: needinfo?(enndeakin)

Neil Deakin

Comment 4

•

5 years ago

I think that is probably ok.

Flags: needinfo?(enndeakin)

Brendan Dahl [:bdahl]

Assignee

Comment 5

•

5 years ago

Attached file Bug 1561326 - Support CSS min/max width/height for top level windows with root HTML element. r=emilio — Details

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

Pulsebot

Comment 6

•

5 years ago

Pushed by bdahl@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/4c31868ffe1c
Support CSS min/max width/height for top level windows with root HTML element. r=emilio

Natalia Csoregi [:nataliaCs]

Comment 7

•

5 years ago

bugherder

https://hg.mozilla.org/mozilla-central/rev/4c31868ffe1c

Status: NEW → RESOLVED

Closed: 5 years ago

status-firefox70: --- → fixed

Resolution: --- → FIXED

Target Milestone: --- → mozilla70

Razvan Maries

Comment 8

•

5 years ago

Backed out for perma fails on browser_startup_syncIPC.js, unexpected PLayerTransaction::Msg_GetTextureFactoryIdentifier sync IPC before first paint.

Push with failure: https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=4c31868ffe1cfe0c9ffbe373bdc249cef70cf5d0&selectedJob=260643664&searchStr=windows%2C10%2Cx64%2Cquantumrender%2Cshippable%2Copt%2Cmochitests%2Ctest-windows10-64-shippable-qr%2Fopt-mochitest-browser-chrome-e10s-3%2Cm%28bc3%29%2C41c90ab516d5d8000141e87dcde22d96dd0c45d6

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 [task 2019-08-08T19:10:11.157Z] 19:10:11 [task 2019-08-08T19:10:11.157Z] 19:10:11 [task 2019-08-08T19:10:11.187Z] 19:10:11 [task 2019-08-08T19:10:11.187Z] 19:10:11 [task 2019-08-08T19:10:11.188Z] 19:10:11 [task 2019-08-08T19:10:11.198Z] 19:10:11 [task 2019-08-08T19:10:11.277Z] 19:10:11 [task 2019-08-08T19:10:11.277Z] 19:10:11 [task 2019-08-08T19:10:11.278Z] 19:10:11 [task 2019-08-08T19:10:11.279Z] 19:10:11 [task 2019-08-08T19:10:11.279Z] 19:10:11 [task 2019-08-08T19:10:11.280Z] 19:10:11 [task 2019-08-08T19:10:11.280Z] 19:10:11 [task 2019-08-08T19:10:11.281Z] 19:10:11 [task 2019-08-08T19:10:11.281Z] 19:10:11 [task 2019-08-08T19:10:11.281Z] 19:10:11 [task 2019-08-08T19:10:11.282Z] 19:10:11 [task 2019-08-08T19:10:11.282Z] 19:10:11 [task 2019-08-08T19:10:11.282Z] 19:10:11 [task 2019-08-08T19:10:11.283Z] 19:10:11 [task 2019-08-08T19:10:11.283Z] 19:10:11 [task 2019-08-08T19:10:11.284Z] 19:10:11 [task 2019-08-08T19:10:11.284Z] 19:10:11 [task 2019-08-08T19:10:11.284Z] 19:10:11 [task 2019-08-08T19:10:11.284Z] 19:10:11 [task 2019-08-08T19:10:11.285Z] 19:10:11 [task 2019-08-08T19:10:11.285Z] 19:10:11 [task 2019-08-08T19:10:11.285Z] 19:10:11 INFO - TEST-START | browser/base/content/test/performance/browser_startup_mainthreadio.js
INFO - GECKO(6956) | MEMORY STAT | vsize 2104173MB | vsizeMaxContiguous 66645103MB | residentFast 283MB | heapAllocated 138MB
INFO - TEST-OK | browser/base/content/test/performance/browser_startup_mainthreadio.js | took 222ms
INFO - checking window state
INFO - GECKO(6956) | JavaScript error: resource://testing-common/PromiseTestUtils.jsm, line 112: uncaught exception: Object
INFO - TEST-START | browser/base/content/test/performance/browser_startup_syncIPC.js
INFO - TEST-INFO | started process screenshot
INFO - TEST-INFO | screenshot: exit 0
INFO - Buffered messages logged at 19:10:11
INFO - Entering test bound
INFO - whitelisted sync IPC before first paint:
INFO - PWebRenderBridge::Msg_EnsureConnected - at most 2 times
INFO - PCompositorBridge::Msg_NotifyChildCreated - at most 2 times
INFO - PCompositorBridge::Msg_MapAndNotifyChildCreated - at most 2 times
INFO - PCompositorBridge::Msg_FlushRendering - at most 1 times
INFO - PCompositorBridge::Msg_Initialize - at most 3 times
INFO - PGPU::Msg_AddLayerTreeIdMapping - at most 5 times
INFO - PCompositorBridge::Msg_WillClose - at most 2 times
INFO - PAPZInputBridge::Msg_ProcessUnhandledEvent - at most 1 times
INFO - PGPU::Msg_GetDeviceStatus - at most 1 times
INFO - Buffered messages finished
INFO - TEST-UNEXPECTED-FAIL | browser/base/content/test/performance/browser_startup_syncIPC.js | unexpected PLayerTransaction::Msg_GetTextureFactoryIdentifier sync IPC before first paint -
INFO - Stack trace:
INFO - chrome://mochikit/content/browser-test.js:test_ok:1576
INFO - chrome://mochitests/content/browser/browser/base/content/test/performance/browser_startup_syncIPC.js:null:317
INFO - chrome://mochikit/content/browser-test.js:Tester_execTest/<:1346
INFO - chrome://mochikit/content/browser-test.js:Tester_execTest:1381
INFO - chrome://mochikit/content/browser-test.js:nextTest/<:1209
INFO - chrome://mochikit/content/tests/SimpleTest/SimpleTest.js:SimpleTest.waitForFocus/waitForFocusInner/focusedOrLoaded/<:803

Backout: https://hg.mozilla.org/mozilla-central/rev/dfac7f4ddfae815ea9077956ce3152bd36944bf6

Status: RESOLVED → REOPENED

Flags: needinfo?(bdahl)

Resolution: FIXED → ---

Target Milestone: mozilla70 → ---

Razvan Maries

Updated

•

5 years ago

status-firefox70: fixed → ---

Brendan Dahl [:bdahl]

Assignee

Comment 9

•

5 years ago

Attached file Bug 1561326 - Lazily constrain window size on Windows. r=mattwoodrow — Details

Avoids IPC to get the max texture size until the layer manager is created.

Depends on D40734

Brendan Dahl [:bdahl]

Assignee

Updated

•

5 years ago

Assignee: nobody → bdahl

Flags: needinfo?(bdahl)

Pulsebot

Comment 10

•

5 years ago

Pushed by bdahl@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/622896be1f0c
Lazily constrain window size on Windows. r=mattwoodrow

Raul Gurzau (:RaulG)

Comment 11

•

5 years ago

bugherder

https://hg.mozilla.org/mozilla-central/rev/622896be1f0c

Status: REOPENED → RESOLVED

Closed: 5 years ago → 5 years ago

status-firefox71: --- → fixed

Resolution: --- → FIXED

Target Milestone: --- → mozilla71

Pulsebot

Comment 12

•

5 years ago

Pushed by bdahl@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/7762fb7ccd1d
Support CSS min/max width/height for top level windows with root HTML element. r=emilio

Raul Gurzau (:RaulG)

Comment 13

•

5 years ago

bugherder

https://hg.mozilla.org/mozilla-central/rev/7762fb7ccd1d

You need to log in before you can comment on or make changes to this bug.

Bugzilla

Quick Search

Support min / max size for top level windows that have <html> root element

Categories

(Core :: Window Management, task, P3)

Tracking

()

People

(Reporter: bgrins, Assigned: bdahl)

References

Details

Crash Data

Security

(public)

User Story

Attachments

(2 files)

Description

Comment 1

Comment 2

Updated

Comment 3

Comment 4

Comment 5

Comment 6

Comment 7

Comment 8

Updated

Comment 9

Updated

Comment 10

Comment 11

Comment 12

Comment 13

Attachment

General

Description

File Name

Content Type