1189554 - Saved passwords dialog is no longer resizable when opened from the Page Info dialog on Windows

Status: VERIFIED FIXED

(Core :: General, defect)

Description

[Matthew N. [:MattN]]

STR:
1) Right-click on a webpage.
2) Click "View Page Info"
3) Click the "Security" tab
4) Click "View Saved Passwords"
5) Saved passwords dialog opens
6) Try to resize the window

Expected result:
The window resizes

Actual result:
The window doesn't resize. Resize cursor doesn't appear.

Last good revision: 6906aba394f7                                      
First bad revision: 310cb0314751                                     
Pushlog:                                                              
https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=6906aba394f7&tochange=310cb0314751 

=> bug 1114299

I think this is related to the arguments being passed to the new dialog window.

This may affect Linux too.

[Tracking Requested - why for this release]: regression

tracking-e10s since an e10s fix regressed this.

Comment 2

[matthew.davis]

I'm using Firefox 40.0 on Linux Mint.

When using the steps above, the window was resizable.
However when going to "Menu -> Preferences -> Security -> Saved Passwords", that window is not resizable. (It appears as part of the page, within the Firefox tab, rather than as an actually separate OS window.)

Comment 3

[Matthew N. [:MattN]]

(In reply to matthew.davis from comment #2)
> When using the steps above, the window was resizable.
> However when going to "Menu -> Preferences -> Security -> Saved Passwords",
> that window is not resizable. (It appears as part of the page, within the
> Firefox tab, rather than as an actually separate OS window.)

That's a separate issue (bug 1176863). I guess this bug only affects Windows (as indicated) but I wasn't sure about Linux. You should be able to resize from the bottom-right corner for the UI you are referring to.

Comment 4

[Mike Conley (:mconley) (:⚙️)]

Going to see if I can at least understand what's happening here today.

Comment 5

[Mike Conley (:mconley) (:⚙️)]

My alterations in bug 1114299 seem to have slightly altered the chromeflags that end up getting computed for the password manager window. That was probably pretty obvious, but now it's confirmed.

Now I need to figure out what the right chromeflags are...

Comment 6

[matthew.davis]

Yes I can confirm that you can resize the window I was talking about by using the bottom right corner.
Thanks MattN.

Comment 7

[Mike Conley (:mconley) (:⚙️)]

Attachment: MozReview Request: Bug 1189554 - Make Saved Passwords dialog resizable on Windows again. r?smaug

Bug 1189554 - Make Saved Passwords dialog resizable on Windows again. r?smaug

We were accidentally overwriting chromeFlags with CHROME_DEFAULT, which
we should only do if the caller has provided a features string when
opening a dialog.

Comment 8

[Olli Pettay [:smaug][[e-mail]]]

Sorry, I don't understand the change. bug 1114299  didn't change those lines of code so why do we need this change?

Comment 9

[Mike Conley (:mconley) (:⚙️)]

(In reply to Olli Pettay [:smaug] from comment #8)
> Sorry, I don't understand the change. bug 1114299  didn't change those lines
> of code so why do we need this change?

Bug 1114299 got rid of a short-circuit that was originally at the top of CalculateChromeFlags that would detect if we were opening a dialog, and then return early:

https://hg.mozilla.org/mozilla-central/annotate/91c69276782a/embedding/components/windowwatcher/nsWindowWatcher.cpp#l1511

I did this because we needed to later logic for adding the remote / non-remote chrome flags as necessary.

Now that we fall through, the conditional I'm changing overwrites the chromeFlags in the original short-circuited case (no features string, opening a dialog).

Comment 10

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/7f5c88364008

Comment 11

[Pulsebot]

Backout:
https://hg.mozilla.org/integration/mozilla-inbound/rev/24d50ddb2235

Comment 12

[Carsten Book [:Tomcat]]

oh turned out that this was innocent for causing a bustage, so we need to check this in again

Comment 13

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/2fb84d5bb9b0

Comment 14

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/2fb84d5bb9b0

Comment 15

[Sylvestre Ledru [:Sylvestre]]

Mike, could you fill the uplift request to beta? Thanks

Comment 16

[Mike Conley (:mconley) (:⚙️)]

Comment on attachment 8661446 [details]
MozReview Request: Bug 1189554 - Make Saved Passwords dialog resizable on Windows again. r?smaug

Approval Request Comment
[Feature/regressing bug #]:

Bug 1114299

[User impact if declined]:

Dialogs that Firefox opens will have the wrong chromeFlags, which can affect things like their resize-ability.

[Describe test coverage new/current, TreeHerder]:

This patch has been in Nightly for quite a few days now.

[Risks and why]: 

It's a small change to the logic of how certain types of windows are opened. Our window opening code is quite complex, but I'm reasonably certain that this reverts us to the original behaviour from before bug 1114299 landed.

[String/UUID change made/needed]:

None.

Comment 17

[Sylvestre Ledru [:Sylvestre]]

Comment on attachment 8661446 [details]
MozReview Request: Bug 1189554 - Make Saved Passwords dialog resizable on Windows again. r?smaug

Thanks, should be in 42 beta 3.

Comment 18

[Sylvestre Ledru [:Sylvestre]]

checkin-need is not necessary for uplifts. :)

Comment 19

[Mike Conley (:mconley) (:⚙️)]

(In reply to Sylvestre Ledru [:sylvestre] from comment #18)
> checkin-need is not necessary for uplifts. :)

Oh okay, so to be clear, I can assume someone else will land these for me?

Comment 20

[Sylvestre Ledru [:Sylvestre]]

The sheriffs are looking for approved uplifts a few times per day. It will land today or tomorrow without you doing anything (except if the patch fails to apply)

Comment 21

[Mike Conley (:mconley) (:⚙️)]

Wonderful, thanks!

Comment 22

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/releases/mozilla-beta/rev/58951da6dffa

Comment 23

[Cornel Ionce [:noni] [Hubs QA]]

Confirming the fix on Windows 7 64bit, Windows 10 32bit and Ubuntu 14.04 using:
* Firefox 42.0b4, build ID: 20151005144425
* Latest 43.0a2 Aurora, build ID: 20151005004046