Closed Bug 1189554 Opened 5 years ago Closed 5 years ago

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

Categories

(Core :: General, defect)

All
Windows
defect
Not set

Tracking

()

VERIFIED FIXED
mozilla43
Tracking Status
e10s - ---
firefox41 --- unaffected
firefox42 + verified
firefox43 --- verified

People

(Reporter: MattN, Assigned: mconley)

References

Details

(Keywords: regression)

Attachments

(1 file)

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.
Flags: firefox-backlog+
Flags: needinfo?(mconley)
Duplicate of this bug: 1194595
(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.
Going to see if I can at least understand what's happening here today.
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...
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.
Attachment #8661446 - Flags: review?(bugs)
Sorry, I don't understand the change. bug 1114299  didn't change those lines of code so why do we need this change?
(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).
Flags: needinfo?(mconley)
Attachment #8661446 - Flags: review?(bugs) → review+
Keywords: checkin-needed
Assignee: nobody → mconley
Status: NEW → ASSIGNED
Component: Preferences → General
Product: Firefox → Core
oh turned out that this was innocent for causing a bustage, so we need to check this in again
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/2fb84d5bb9b0
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla43
Mike, could you fill the uplift request to beta? Thanks
Flags: needinfo?(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.
Flags: needinfo?(mconley)
Attachment #8661446 - Flags: approval-mozilla-beta?
Attachment #8661446 - Flags: approval-mozilla-aurora?
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.
Attachment #8661446 - Flags: approval-mozilla-beta?
Attachment #8661446 - Flags: approval-mozilla-beta+
Attachment #8661446 - Flags: approval-mozilla-aurora?
Attachment #8661446 - Flags: approval-mozilla-aurora+
Keywords: checkin-needed
checkin-need is not necessary for uplifts. :)
Keywords: checkin-needed
(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?
Flags: needinfo?(sledru)
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)
Flags: needinfo?(sledru)
Wonderful, thanks!
Flags: qe-verify+
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
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.