Closed Bug 1600281 Opened 4 months ago Closed 4 months ago

Strange alignment of container buttons in preferences

Categories

(Firefox :: Preferences, defect, P1, major)

defect

Tracking

()

VERIFIED FIXED
Firefox 73
Root Cause ?
Tracking Status
firefox-esr68 --- unaffected
firefox70 --- unaffected
firefox71 --- unaffected
firefox72 blocking verified
firefox73 --- verified

People

(Reporter: ntim, Assigned: bgrins)

References

(Regression)

Details

(Keywords: regression)

Attachments

(2 files)

"Preferences" and "Remove" buttons are weirdly aligned.

STR: visit about:preferences#containers

Attached image Screenshot

Removing https://searchfox.org/mozilla-central/source/browser/components/preferences/in-content/containers.js#67 would fix this, but it'd be nice to check if anything else is affected by this.

Brian, could you please take a look ?

Flags: needinfo?(bgrinstead)

Oh, I see what's wrong. The "Rewrite [align="left"] to [align="start"] and [align="right"] to [align="end"]" patch is wrong.

align="left" maps to pack="start" for horizontal boxes (-moz-box-orient: horizontal) and align="start" for vertical boxes.
Similarly, align="right" maps to pack="end" for horizontal boxes and align="end" for vertical boxes.

See: https://hg.mozilla.org/mozilla-central/rev/c58aa1b00988#l1.89

It would be nice to re-review all the files changed in https://hg.mozilla.org/mozilla-central/rev/c59883012b35 to make sure nothing else regressed.

[Tracking Requested - why for this release]:
User-visible regression that we'll need to fix for release.

Priority: -- → P1

(In reply to Tim Nguyen :ntim from comment #5)

Oh, I see what's wrong. The "Rewrite [align="left"] to [align="start"] and [align="right"] to [align="end"]" patch is wrong.

align="left" maps to pack="start" for horizontal boxes (-moz-box-orient: horizontal) and align="start" for vertical boxes.
Similarly, align="right" maps to pack="end" for horizontal boxes and align="end" for vertical boxes.

See: https://hg.mozilla.org/mozilla-central/rev/c58aa1b00988#l1.89

It would be nice to re-review all the files changed in https://hg.mozilla.org/mozilla-central/rev/c59883012b35 to make sure nothing else regressed.

Thanks, I'll have a look.

I guess the same issue could have happened with the change from align="top" -> align="start" in Bug 1599283, but the only elements that got updated in https://hg.mozilla.org/integration/autoland/rev/ad75d8bb97ca are horizontally aligned so it uses the align attribute for valignment: https://hg.mozilla.org/mozilla-central/rev/c58aa1b00988#l1.89.

Assignee: nobody → bgrinstead
Status: NEW → ASSIGNED
Flags: needinfo?(bgrinstead)

(In reply to Brian Grinstead [:bgrins] from comment #7)

I guess the same issue could have happened with the change from align="top" -> align="start" in Bug 1599283, but the only elements that got updated in https://hg.mozilla.org/integration/autoland/rev/ad75d8bb97ca are horizontally aligned so it uses the align attribute for valignment: https://hg.mozilla.org/mozilla-central/rev/c58aa1b00988#l1.89.

I think I misread this. align=top would have hit the early return at https://hg.mozilla.org/mozilla-central/rev/c58aa1b00988#l1.77 so pack would never be used.

  • align="left" maps to pack="start" for horizontal boxes and align="start" for vertical boxes.
  • align="right" maps to pack="end" for horizontal boxes and align="end" for vertical boxes.
Pushed by bgrinstead@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/9ed78435ef6d
Rewrite horizontal boxes that were incorrectly translated from align=right to align=end to use pack=end r=Gijs
Status: ASSIGNED → RESOLVED
Closed: 4 months ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 73
Flags: qe-verify+

Verified as fixed on Firefox 73.0a1 (2019-12-03) on Windows 10 x 64, Mac OS X 10.14 and on Ubuntu 18.04 x64.

Comment on attachment 9112987 [details]
Bug 1600281 - Rewrite horizontal boxes that were incorrectly translated from align=right to align=end to use pack=end

Beta/Release Uplift Approval Request

  • User impact if declined: Various elements are misaligned in the UI (attachment 9112522 [details], attachment 9111390 [details])
  • Is this code covered by automated tests?: No
  • Has the fix been verified in Nightly?: Yes
  • Needs manual test from QE?:
  • If yes, steps to reproduce:
  • List of other uplifts needed: None
  • Risk to taking this patch: Low
  • Why is the change risky/not risky? (and alternatives if risky): Straightforward XUL patch
  • String changes made/needed:
Attachment #9112987 - Flags: approval-mozilla-beta?

This might be worth porting to TB, summary is in comment 5 :)

Flags: needinfo?(richard.marti)
Flags: needinfo?(mkmelin+mozilla)

Thx, Richard already noticed and has a patch in bug 1600928.

Flags: needinfo?(richard.marti)
Flags: needinfo?(mkmelin+mozilla)
QA Whiteboard: [qa-triaged]

Comment on attachment 9112987 [details]
Bug 1600281 - Rewrite horizontal boxes that were incorrectly translated from align=right to align=end to use pack=end

fix a new regression in preferences, verified in 73, approved for 72.0b3

Attachment #9112987 - Flags: approval-mozilla-beta? → approval-mozilla-beta+

Verified as fixed on Firefox 72.0b3 (Build ID: 20191204104712) on Windows 10 x 64, Mac OS X 10.14 and on Ubuntu 18.04 x64.

Status: RESOLVED → VERIFIED
QA Whiteboard: [qa-triaged]
Flags: qe-verify+
Regressions: 1602947

Please specify a root cause for this bug. See :tmaity for more information.

Root Cause: --- → ?
You need to log in before you can comment on or make changes to this bug.