1601811 - Replace writing window.inner{Width,Height} with something others

Status: ASSIGNED

(MailNews Core :: Account Manager, task)

Description

[Hiroyuki Ikezoe (:hiro)]

I was going to make window.inner{Width,Hegith} compliant with the spec in bug 1595962, but some comm-central codes do write the property directly and expect that it changes window size too.

Instead changing the property we can use window.resizeBy() or resizeTo().

Comment 1

[Hiroyuki Ikezoe (:hiro)]

Attachment: Use window.resizeBy() instead of changing window.inner{Width,Height}

Comment 2

[:aceman]

Comment on attachment 9113997 [details] [diff] [review]
Use window.resizeBy() instead of changing window.inner{Width,Height}

Thanks for fixing this for us beforehand.
Did you merge it into one winwo.resizeby() call so that the window only resizes once to the final dimensions, not in 2 steps?

Comment 3

[Magnus Melin [:mkmelin]]

At least on trunk the "secure warning" is apparently broken :(

So the patch as such can't be tested. https://searchfox.org/comm-central/rev/bc17d0ef9860233e9ef72dc9324221ba86d662ae/mail/components/accountcreation/content/emailWizard.js#2258 needs to be gEmailConfigWizard.resizeDialog()

Comment 4

[Magnus Melin [:mkmelin]]

Bug 1604409 fixed the brokenness from comment 3.

Comment 5

[Magnus Melin [:mkmelin]]

Attachment: bug1601811_window_resize.patch

Updated to trunk.
I'm not sure it works, the whole function I mean. For manual resize it never gets out of the too small resized state. Didn't try with l10n yet.

Comment 6

[Ben Bucksch (:BenB)]

What do you mean with "manual resize"?

What actual bug is motivating the code changes here?
And since when does that bug exist?

[Removed comment about Bug 1604409]

Comment 7

[Magnus Melin [:mkmelin]]

That if I manually resize the dialog to something uncomfortable, later when onResize is called it doesn't get big enough to show everything without scrollbars.
Just trying to understand https://searchfox.org/comm-central/rev/7d24f30cebfef878d2bf89dfd2305753d8e7d481/mail/components/accountcreation/content/emailWizard.js#156-157

To be clear, content is still accessible since there are the scrollbars. Just not comfy.

What actual bug is motivating the code changes here?

Bug 1595962

Comment 8

[Ben Bucksch (:BenB)]

I see.

manually resize the dialog to something uncomfortable

I presume you mean something smaller than necessary for the window contents.

FWIW, the manual resize case is not really important. It's much more important to get the size correct automatically. If the user manually resizes, and thjen later the window content changes, and we're automatically resizing smaller again, I would not mind that.

Could you please try to use resizeTo() instead of resizeBy()? Even if it doesn't fix the bug you're seeing, it still better matches the intent of the code.

I think it does make sense to make the window smaller as well. Esp. after leaving the a warning box.

Comment 9

[Magnus Melin [:mkmelin]]

Maybe, but it's not that window.resizeBy doesn't "work". It's that we don't seem to get to the case where we (would) call it.