1443864 - Apply size constraints in nsXULWindow too.

Status: RESOLVED FIXED

(Core :: XUL, enhancement)

Description

[Emilio Cobos Álvarez (:emilio)]

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.

Comment 1

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1443864: Apply size constraints on nsXULWindow too.

Review commit: https://reviewboard.mozilla.org/r/225854/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/225854/

Comment 2

[Emilio Cobos Álvarez (:emilio)]

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

Comment 3

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1443864: Maybe a bit cleaner.

I don't feel too strongly about this patch. If you think it's worth, I'll merge
it with the patch above.

Review commit: https://reviewboard.mozilla.org/r/225874/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/225874/

Comment 4

[Boris Zbarsky [: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.

Comment 5

[Boris Zbarsky [:bzbarsky]]

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.

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/9954a9ffb537
Apply size constraints on nsXULWindow too. r=bz

Comment 7

[Andreea Pavel [:apavel]]

https://hg.mozilla.org/mozilla-central/rev/9954a9ffb537