Closed Bug 1271502 Opened 8 years ago Closed 8 years ago

list does not align well in private browsing new tab page with locale lang

Categories

(Firefox :: Private Browsing, defect)

48 Branch
defect
Not set
normal

Tracking

()

RESOLVED FIXED
Firefox 49
Tracking Status
firefox47 --- unaffected
firefox48 --- fixed
firefox49 --- fixed

People

(Reporter: gasolin, Assigned: gasolin)

References

Details

(Keywords: regression)

Attachments

(2 files)

      No description provided.
Summary: list does not in private browsing new tab page → list does not align well in private browsing new tab page with locale lang
Attached image text_not_align_well.png
Tested with Developer Edition (48) on mac and linux, 

The reason is the font height does not the same in English (height: 20px) and Chinese(height: 24px) at first row. It makes the overflow rule render an unexpected layout.
Assignee: nobody → gasolin
Comment on attachment 8750576 [details]
MozReview Request: Bug 1271502 - list does not align well in private browsing new tab page with locale lang; r?Gijs

Ricky, could you help take a look for layout in different locale?
Attachment #8750576 - Flags: feedback?(rchien)
Please remember to request uplift once this lands on master.
Blocks: 1259340
https://reviewboard.mozilla.org/r/51483/#review48653

Thank you Fred! It works on my developer edition. However, I'm wondering why it doesn't happen when trying to replace english word to chinese in en-US developer edition?

::: browser/themes/shared/privatebrowsing/aboutPrivateBrowsing.css:69
(Diff revision 1)
>  p {
>    line-height: 1.5em;
>  }
>  
>  .list-row {
> -  overflow: auto;
> +  overflow: hidden;

What is this used for?
I didn't see anything different after applying this line.
Attachment #8750576 - Flags: feedback?(rchien) → feedback+
(In reply to Ricky Chien [:rickychien] from comment #5)
> https://reviewboard.mozilla.org/r/51483/#review48653
> 
> Thank you Fred! It works on my developer edition. However, I'm wondering why
> it doesn't happen when trying to replace english word to chinese in en-US
> developer edition?
> 
> ::: browser/themes/shared/privatebrowsing/aboutPrivateBrowsing.css:69
> (Diff revision 1)
> >  p {
> >    line-height: 1.5em;
> >  }
> >  
> >  .list-row {
> > -  overflow: auto;
> > +  overflow: hidden;
> 
> What is this used for?
> I didn't see anything different after applying this line.

When I set the height for li item, I got unexpected scrollbar at right of the list-row.
I think scrollbar is unnecessary for a list row.
Attachment #8750576 - Flags: review?(gijskruitbosch+bugs)
Comment on attachment 8750576 [details]
MozReview Request: Bug 1271502 - list does not align well in private browsing new tab page with locale lang; r?Gijs

This is going to break if the text in the <li>s ever wraps, which it might in other locales / different window sizes, because their width is limited. 

We should set line-height instead. Likewise, I don't think the change to overflow: auto is necessary - it seems likely to lead to clipping instead.
Attachment #8750576 - Flags: review?(gijskruitbosch+bugs)
Comment on attachment 8750576 [details]
MozReview Request: Bug 1271502 - list does not align well in private browsing new tab page with locale lang; r?Gijs

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/51483/diff/1-2/
Attachment #8750576 - Attachment description: MozReview Request: Bug 1271502 - list does not align well in private browsing new tab page with locale lang; → MozReview Request: Bug 1271502 - list does not align well in private browsing new tab page with locale lang; r?Gijs
Attachment #8750576 - Flags: feedback+ → review?(gijskruitbosch+bugs)
Attachment #8750576 - Flags: review?(gijskruitbosch+bugs) → review+
Comment on attachment 8750576 [details]
MozReview Request: Bug 1271502 - list does not align well in private browsing new tab page with locale lang; r?Gijs

https://reviewboard.mozilla.org/r/51483/#review48705
Thanks for review. I've change height to line-height and it works well.
Also turn `margin-left` in list-row item to `-moz-margin-start`
Keywords: checkin-needed
Comment on attachment 8750576 [details]
MozReview Request: Bug 1271502 - list does not align well in private browsing new tab page with locale lang; r?Gijs

Approval Request Comment
[Feature/regressing bug #]: bug 1259340
[User impact if declined]: the list in private browsing new tab page not shown correctly in locale language (test in chinese)
[Describe test coverage new/current, TreeHerder]: N
[Risks and why]: CSS only fix
[String/UUID change made/needed]: N
Attachment #8750576 - Flags: approval-mozilla-aurora?
https://hg.mozilla.org/mozilla-central/rev/d450d458432d
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 49
Minor layout regression from 48.
Comment on attachment 8750576 [details]
MozReview Request: Bug 1271502 - list does not align well in private browsing new tab page with locale lang; r?Gijs

CSS fix for new tab layout regression in 48. Please uplift to aurora.
Attachment #8750576 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
You need to log in before you can comment on or make changes to this bug.