Maybe a dupe of bug 1214064. STR: 1) Open about:preferences#security 2) Click on "Saved Logins" 3) Resize and close the window 4) Click again on "Saved Logins" Result: the window has its default size. Many windows of about:preferences are affected, the user should be able to keep a size of his choice for these windows, especially for saved logins. That was always the case in the past.

Reg range: https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=44904f46bc865b5a6272842e520252171ea4e404&tochange=0136029e390e363e61540205b9902dcb9f3cb3ec

I think this is actually a front-end bug, about:preferences is in-content now, so I'd expect the front-end code there would need to explicitly save the pseudo-window sizes. (This happened automatically for XUL windows, but we're not using those when it's in-content.).

(How is bug 1137009 even involved here? Do we believe the regression range?)

Bug 1043346 cleared the sizes of dialogs when they were closed (Firefox 37) so one resized subdialog wouldn't affect the size of the next opened subdialog even if they were different subdialogs. Then bug 1043612 and bug 1153403 persisted the sizes of dialogs when they were closed (Firefox 40) so the next time the same subdialog is reopened it would reopen at the previous size. In bug 1043612 we used XUL attribute persistence to store these. Regrettably that patch didn't introduce an automated test.

Given comment 4, I know how to fix it. But I don't have time to investigate how to write the test, so if anyone can help here, it would be appreciated.

The funny thing is some windows are not affected. In about:preferences#security, the first "Exceptions" window is affected (size not saved) but not the 2nd "Exceptions" window.

You may be able to look at /browser/components/preferences/in-content/tests/browser_subdialogs.js to see how tests can open subdialogs. Note that browser_subdialogs.js uses a generic subdialog, not the ones that are actually opened in the real preferences.

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

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

(In reply to Jared Wein [:jaws] (please needinfo? me) from comment #7) > You may be able to look at > /browser/components/preferences/in-content/tests/browser_subdialogs.js to > see how tests can open subdialogs. Note that browser_subdialogs.js uses a > generic subdialog, not the ones that are actually opened in the real > preferences. Then it is actually not testable. This issue happens because dialogs in preferences use <xul:window> for its content. I manually tested the patchset, and it seems it works.

Comment on attachment 8742609 [details] MozReview Request: Bug 1264968 part 1 - Use replace uses of removed String.contains with the new .includes method. r=jaws https://reviewboard.mozilla.org/r/47351/#review44059 A test here would have prevented this from silently breaking. Did you take a look at browser_subdialogs.js?

Sorry I didn't see comment 10. That doesn't explain the switch from .contains to .includes though. We could also change subdialog.xul to use a <xul:window>

(In reply to Jared Wein [:jaws] (please needinfo? me) from comment #12) > Sorry I didn't see comment 10. That doesn't explain the switch from > .contains to .includes though. Part 2 is the real fix, but part 1 probably also involves since it stops setting width/height attributes unconditionally. > We could also change subdialog.xul to use a <xul:window> Hmmm, probably...

But why was the test designed to use <dialog> from the very first?

Some of our "dialogs" use <dialog>, see http://mxr.mozilla.org/mozilla-central/source/toolkit/mozapps/preferences/changemp.xul#16. The difference has been negligible to this point and I believe <dialog> was chosen because of semantics and not expecting behavioral differences.

(In reply to Jared Wein [:jaws] (please needinfo? me) from comment #15) > Some of our "dialogs" use <dialog>, see > http://mxr.mozilla.org/mozilla-central/source/toolkit/mozapps/preferences/ > changemp.xul#16. The difference has been negligible to this point and I > believe <dialog> was chosen because of semantics and not expecting > behavioral differences. Well, then I guess another way to fix this issue is, changing every <window> to <dialog>, then. Since only <window> was broken on persist due to the assumption that it is only used for top level window. That also says, even if there was a test, the test would not catch this regression either.

Matt, do you recall why <dialog> was used for the test?

(In reply to Loic from comment #6) > The funny thing is some windows are not affected. > In about:preferences#security, the first "Exceptions" window is affected > (size not saved) but not the 2nd "Exceptions" window. The second "Exceptions" window uses <prefwindow> rather than <window>. It seems there are roughly same number of <window> and <prefwindow> uses. Probably we can consider converting all <window> to <prefwindow> to fix this issue rather than change the C++ code.

It seems <prefwindow> and <window> does have some behavioral differences. :jaws, could you take this bug and try fixing it via converting all <window> on pref subdialogs to something different? <dialog> and <prefwindow> should both work. I'm not quite familiar with all these xul things... And I suppose making them consistent is a good thing itself.

I don't have the time right now to make and test that change. Some dialogs may be opened outside of the preferences in SeaMonkey or other programs.

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

Comment on attachment 8742644 [details] MozReview Request: Bug 1264968 part 2 - Use <dialog> rather than <window> for pref subdialogs. r?jaws https://reviewboard.mozilla.org/r/47391/#review44169 The Saved Logins dialog is also reachable from right-clicking on a login form and selecting Fill Login > View Saved Logins. This change adds an OK/Cancel to that dialog, http://screencast.com/t/sUZ1F0u1

(In reply to Jared Wein [:jaws] (please needinfo? me) from comment #17) > Matt, do you recall why <dialog> was used for the test? Probably like you said because it made sense semantically but I don't remember off-hand. It seems like we should be testing all of the types of documentElements that we use in prefs.

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

Comment on attachment 8742609 [details] MozReview Request: Bug 1264968 part 1 - Use replace uses of removed String.contains with the new .includes method. r=jaws Review request updated; see interdiff: https://reviewboard.mozilla.org/r/47351/diff/1-2/

Comment on attachment 8744176 [details] MozReview Request: Bug 1264968 part 2 - Allow persisting attributes of xul:window if its owner document is not root. r?enn https://reviewboard.mozilla.org/r/48363/#review45173

https://hg.mozilla.org/integration/mozilla-inbound/rev/3663fe230f72ac37afbc25996cca6f2ef8794a4d Bug 1264968 part 1 - Use replace uses of removed String.contains with the new .includes method. r=jaws https://hg.mozilla.org/integration/mozilla-inbound/rev/660b322ef9fd22614359864cb2ebff3943511766 Bug 1264968 part 2 - Allow persisting attributes of xul:window if its owner document is not root. r=enndeakin

https://hg.mozilla.org/mozilla-central/rev/3663fe230f72 https://hg.mozilla.org/mozilla-central/rev/660b322ef9fd

I have reproduced this bug on Nightly 48.0a1 (2016-04-15) on ubuntu 16.04 LTS, 64 bit! The bug's fix is now verified on Latest Nightly 49.0a1! Build ID: 20160518030234 User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:49.0) Gecko/20100101 Firefox/49.0

Reproduced this bug with Firefox nightly 48.0a1(build id:20160424030601)on windows 7(64 bit) Verified this bug as fixed with Firefox beta 48.0b3(build id:20160623122823) User Agent:Mozilla/5.0 (Windows NT 6.1; WOW64; rv:48.0) Gecko/20100101 Firefox/48.0 Verified as fixed with Firefox aurora 49.0a2(build id:20160723004004) User Agent:Mozilla/5.0 (Windows NT 6.1; WOW64; rv:49.0) Gecko/20100101 Firefox/49.0 [testday-20160722]

As the bug is verified on firefox 48, firefox 49 by linux(Comment 29) and windows(Comment 30) marking as verified