Closed Bug 1443864 Opened 2 years ago Closed 2 years ago

Apply size constraints in nsXULWindow too.

Categories

(Core :: XUL, enhancement)

enhancement
Not set

Tracking

()

RESOLVED FIXED
mozilla61
Tracking Status
firefox61 --- fixed

People

(Reporter: emilio, Assigned: emilio)

References

Details

Attachments

(2 files)

If we're going to size the window during layout, we may as well avoid spurious resizes after layout.

Also, not all widgets play nice with setting constraints on windows without a later resize, so this should make the patch in bug 1439875 less risky.
(In reply to Emilio Cobos Álvarez [:emilio] from comment #0)
> If we're going to size the window during layout [..]

Err, this should read "before layout" of course.
Attachment #8956897 - Flags: review?(xidorn+moz) → review?(bzbarsky)
Attachment #8956909 - Flags: review?(xidorn+moz) → review?(bzbarsky)
Comment on attachment 8956897 [details]
Bug 1443864: Apply size constraints on nsXULWindow too.

https://reviewboard.mozilla.org/r/225854/#review232820

r=me with the issues below fixed.

::: xpfe/appshell/nsXULWindow.cpp:1288
(Diff revision 1)
> -  nsAutoString sizeString;
>  
> -  windowElement->GetAttribute(WIDTH_ATTRIBUTE, sizeString);
> -  temp = sizeString.ToInteger(&errorCode);
> -  if (NS_SUCCEEDED(errorCode) && temp > 0) {
> -    aSpecWidth = std::max(temp, 100);
> +  if (auto width = ReadIntAttribute(*windowElement, nsGkAtoms::width)) {
> +    int32_t min =
> +      ReadIntAttribute(*windowElement, nsGkAtoms::minwidth)
> +        .valueOr(100);

This changes behavior if minwidth is specified and set less than 100, no?  Please preserve the old behavior.

::: xpfe/appshell/nsXULWindow.cpp:1299
(Diff revision 1)
>      gotSize = true;
>    }
> -  windowElement->GetAttribute(HEIGHT_ATTRIBUTE, sizeString);
> -  temp = sizeString.ToInteger(&errorCode);
> -  if (NS_SUCCEEDED(errorCode) && temp > 0) {
> -    aSpecHeight = std::max(temp, 100);
> +
> +  if (auto height = ReadIntAttribute(*windowElement, nsGkAtoms::height)) {
> +    int32_t min =
> +      ReadIntAttribute(*windowElement, nsGkAtoms::minheight)

Again, this changes behavior if minheight < 100.
Attachment #8956897 - Flags: review?(bzbarsky) → review+
Comment on attachment 8956909 [details]
Bug 1443864: Maybe a bit cleaner.

https://reviewboard.mozilla.org/r/225874/#review232822

r=me subject to the same comments about specified minwidth/height < 100.
Attachment #8956909 - Flags: review?(bzbarsky) → review+
Pushed by ecoal95@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/9954a9ffb537
Apply size constraints on nsXULWindow too. r=bz
https://hg.mozilla.org/mozilla-central/rev/9954a9ffb537
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla61
You need to log in before you can comment on or make changes to this bug.