1425822 - "Settings for Clearing History" column width is still off in non-en-US locales

Status: RESOLVED FIXED

(Firefox :: Settings UI, defect)

Description

[Francesco Lodolo [:flod]]

Attachment: Screenshot comparing 58 and 59 (Italian)

See the screenshot that compares the same dialog, on an Italian build, between current Nightly and Beta (don't have a regression range).

Issues:
* Columns are uneven.
* Title is displayed in a smaller font than other text.
* (pre-existing) Dialog is not resizable, which leads to issues like bug 1322955.

Comment 1

[Dão Gottwald [:dao]]

(In reply to Francesco Lodolo [:flod] from comment #0)
> Created attachment 8937414 [details]
> Screenshot comparing 58 and 59 (Italian)
> 
> See the screenshot that compares the same dialog, on an Italian build,
> between current Nightly and Beta (don't have a regression range).
> 
> Issues:
> * Columns are uneven.

Short of determining the column width automatically which I think would be preferable, this needs to be fixed in the locale. The em value from Firefox 58 is referring to root font size which doesn't make much sense. Maybe we should change the entity name so locals are forced to update this.

> * Title is displayed in a smaller font than other text.

I don't see that in your screenshot. In fact the title looks smaller in 58.

Comment 2

[Dão Gottwald [:dao]]

(In reply to Dão Gottwald [::dao] from comment #1)
> > * Columns are uneven.
> 
> Short of determining the column width automatically which I think would be
> preferable, this needs to be fixed in the locale. The em value from Firefox
> 58 is referring to root font size which doesn't make much sense. Maybe we
> should change the entity name so locals are forced to update this.

Actually it looks like we should just get rid of sanitizePrefs2.inContent.column.width at this point. Its value is the same as sanitizePrefs2.column.width, which makes sense.

Comment 3

[Francesco Lodolo [:flod]]

Attachment: Screenshot of 59

Screenshot with section titles highlighted.

Comment 4

[Francesco Lodolo [:flod]]

(In reply to Dão Gottwald [::dao] from comment #1)
> I don't see that in your screenshot. In fact the title looks smaller in 58.

"title" might have been misleading, I meant the section title above each group of checkboxes.

Comment 5

[Dão Gottwald [:dao]]

Attachment: Bug 1425822 - Get rid of sanitizePrefs2.inContent.dialog.width and sanitizePrefs2.inContent.column.width.

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

Comment 6

[Dão Gottwald [:dao]]

(In reply to Francesco Lodolo [:flod] from comment #4)
> (In reply to Dão Gottwald [::dao] from comment #1)
> > I don't see that in your screenshot. In fact the title looks smaller in 58.
> 
> "title" might have been misleading, I meant the section title above each
> group of checkboxes.

Okay, this is probably Mac-specific. Should be filed as a separate bug.

Comment 7

[Dão Gottwald [:dao]]

Comment on attachment 8937967 [details]
Bug 1425822 - Get rid of sanitizePrefs2.inContent.dialog.width and sanitizePrefs2.inContent.column.width.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/208706/diff/1-2/

Comment 8

[Francesco Lodolo [:flod]]

Just to make sure we don't fall into the same issue that requested a back out in bug 1425724.

sanitizePrefs2.modal.width and sanitizePrefs2.column.width are currently unused?

The safer approach would be to use two new strings for these values, and drop the 4 old ones. These two strings were customized for the old preferences, and this change might lead to unexpected results
https://transvision.mozfr.org/string/?entity=browser/chrome/browser/sanitize.dtd:sanitizePrefs2.inContent.column.width&repo=gecko_strings
https://transvision.mozfr.org/string/?entity=browser/chrome/browser/sanitize.dtd:sanitizePrefs2.inContent.dialog.width&repo=gecko_strings

Comment 9

[Dão Gottwald [:dao]]

(In reply to Francesco Lodolo [:flod] from comment #8)
> These two strings were customized for the old
> preferences, and this change might lead to unexpected results

In the non-in-content dialog, the em value was relative to the actual text size, just like today as of bug 1422100.

Comment 10

[Francesco Lodolo [:flod]]

(In reply to Dão Gottwald [::dao] from comment #9)
> (In reply to Francesco Lodolo [:flod] from comment #8)
> > These two strings were customized for the old
> > preferences, and this change might lead to unexpected results
> 
> In the non-in-content dialog, the em value was relative to the actual text
> size, just like today as of bug 1422100.

Thanks for confirming. No need to change string IDs then.

Comment 11

[Jared Wein [:jaws] (please needinfo? me)]

Comment on attachment 8937967 [details]
Bug 1425822 - Get rid of sanitizePrefs2.inContent.dialog.width and sanitizePrefs2.inContent.column.width.

https://reviewboard.mozilla.org/r/208706/#review214916

Comment 12

[Mozilla Autoland]

We're sorry, Autoland could not rebase your commits for you automatically. Please manually rebase your commits and try again.

hg error in cmd: hg rebase -s 9292e549f38b -d b5b951c9a615: rebasing 440067:9292e549f38b "Bug 1425822 - Get rid of sanitizePrefs2.inContent.dialog.width and sanitizePrefs2.inContent.column.width. r=jaws" (tip)
merging browser/locales/en-US/chrome/browser/sanitize.dtd
warning: conflicts while merging browser/locales/en-US/chrome/browser/sanitize.dtd! (edit, then use 'hg resolve --mark')
unresolved conflicts (see hg resolve, then hg rebase --continue)

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/4db35709292a
Get rid of sanitizePrefs2.inContent.dialog.width and sanitizePrefs2.inContent.column.width. r=jaws

Comment 14

[Stefan Hindli [:stefan_hindli]]

https://hg.mozilla.org/mozilla-central/rev/4db35709292a