Convert privacy.js to use gSubDialog.

To look correct it needs bug 1035540 applied first.

Comment on attachment 8452601 [details] [diff] [review] privacyDialogsIncontent.patch Review of attachment 8452601 [details] [diff] [review]: ----------------------------------------------------------------- ::: browser/components/preferences/in-content/privacy.js @@ +477,5 @@ > * Displays all the user's cookies in a dialog. > */ > showCookies: function (aCategory) > { > + gSubDialog.open("chrome://browser/content/preferences/cookies.xul"); This dialog doesn't have enough padding at the bottom on OS X. Maybe add padding-bottom: 6px to #dialogBox or figure out why this dialog is different and make a better fix. f+ for now but I can be convinced to r+ if there is a bigger plan to fix this. @@ +503,5 @@ > /** > * Displays a dialog from which individual parts of private data may be > * cleared. > */ > clearPrivateDataNow: function (aClearEverything) Since you're not doing it here, can you get a bug on file about making this dialog in-content. You can trigger it when "Remember History" is specified and you click the "clear your recent history" link.

(In reply to Matthew N. [:MattN] (behind on bugmail) from comment #2) > Comment on attachment 8452601 [details] [diff] [review] > privacyDialogsIncontent.patch > > Review of attachment 8452601 [details] [diff] [review]: > ----------------------------------------------------------------- > > ::: browser/components/preferences/in-content/privacy.js > @@ +477,5 @@ > > * Displays all the user's cookies in a dialog. > > */ > > showCookies: function (aCategory) > > { > > + gSubDialog.open("chrome://browser/content/preferences/cookies.xul"); > > This dialog doesn't have enough padding at the bottom on OS X. Maybe add > padding-bottom: 6px to #dialogBox or figure out why this dialog is different > and make a better fix. f+ for now but I can be convinced to r+ if there is a > bigger plan to fix this. This is really a strange beast. On Linux and Windows the height is correctly calculated but OS X needs this additional padding. For me this is a bit a hack but I don't know why only OS X calculates this differently and how to fix this without the padding-bottom I added to the OS X file.. > @@ +503,5 @@ > > /** > > * Displays a dialog from which individual parts of private data may be > > * cleared. > > */ > > clearPrivateDataNow: function (aClearEverything) > > Since you're not doing it here, can you get a bug on file about making this > dialog in-content. You can trigger it when "Remember History" is specified > and you click the "clear your recent history" link. Filed bug 1036595.

Comment on attachment 8453271 [details] [diff] [review] privacyDialogsIncontent.patch Review of attachment 8453271 [details] [diff] [review]: ----------------------------------------------------------------- Dão, any chance you could try find some cleaner CSS for Richard that isn't specific to one dialog? I haven't had time to look at this due to search/Loop stuff.

Comment on attachment 8453271 [details] [diff] [review] privacyDialogsIncontent.patch Review of attachment 8453271 [details] [diff] [review]: ----------------------------------------------------------------- r=me without the CSS change by adding the missing padding-bottom in the height calculation instead. I'll make this change and land it now. ::: browser/themes/osx/preferences/in-content/preferences.css @@ +96,5 @@ > } > + > +/* Add to Cookies dialog a bottom padding */ > +#CookiesDialog > hbox > .actionButtons { > + padding-bottom: 10px; It seems like this is only necessary since the padding-bottom of #CookiesDialog isn't included in the height calculation of the <browser> for some reason despite MDN saying the padding should be included in scrollHeight. It seems like we should fix the height calculation JS to include padding instead of hard-coding this. I confirmed with a reduced testcase that the padding-top is included in scrollHeight but padding-bottom is not. The same applies to <prefwindow> so I think it applies to any document element. It's not clear to me whether this is a bug in XUL or not since there is no real spec.

Reproduced bug on older build Mozilla/5.0 (X11; Linux x86_64; rv:33.0) Gecko/20100101 Firefox/33.0 I cannot confirm fix on latest Nightly Mozilla/5.0 (X11; Linux x86_64; rv:34.0) Gecko/20100101 Firefox/34.0 because even though the individual cookies setting opens an in-content dialog, the "clear your recent history" link is not currently opening an in-content dialog. Richard, is this expected?

This is true and will be handled in bug 1036595.

(In reply to Richard Marti (:Paenglab) from comment #9) > This is true and will be handled in bug 1036595. Ok, then as per my last comment, pending verification in another platform I recommend this bug to be set as VERIFIED. Since this bug is marked as fixed, I've changed title to better reflect what is fixed in this bug.

Clicking "remove individual cookies" on about:preferences#privacy open in-content on Mozilla/5.0 (Windows NT 6.1; WOW64; rv:34.0) Gecko/20100101 Firefox/34.0 ID:20140820030202 CSet: ffdd1a398105