Closed
Bug 1167616
Opened 9 years ago
Closed 9 years ago
Change Search Settings panel is sized incorrectly on new tab page
Categories
(Firefox :: New Tab Page, defect)
Firefox
New Tab Page
Tracking
()
Tracking | Status | |
---|---|---|
firefox38 | --- | unaffected |
firefox38.0.5 | --- | unaffected |
firefox39 | --- | wontfix |
firefox40 | --- | verified |
firefox41 | --- | fixed |
firefox42 | --- | unaffected |
firefox43 | --- | unaffected |
firefox-esr31 | --- | unaffected |
firefox-esr38 | --- | unaffected |
People
(Reporter: mconley, Assigned: ursula)
References
(Blocks 1 open bug)
Details
(Whiteboard: [Bugday-20150624])
Attachments
(3 files, 6 obsolete files)
40.54 KB,
image/png
|
Details | |
308.82 KB,
image/png
|
Details | |
1.21 KB,
patch
|
ursula
:
review+
ritu
:
approval-mozilla-beta+
|
Details | Diff | Splinter Review |
STR: 1) Go to about:newtab 2) Click on the magnifying glass icon to the left of the search input ER: A panel should open with UI to send the user to the Search Settings preferences. AR: A panel opens with the UI, but the UI is kind of cut off. See screenshot.
Reporter | ||
Comment 1•9 years ago
|
||
Comment 2•9 years ago
|
||
I also see this on Windows 7 & 8. It looks to have been introduced with Firefox 39. Firefox 38.0.5 and earlier don't have the bug. It's also worse on Nightly & Aurora as compared to Beta 39.
Updated•9 years ago
|
status-firefox38:
--- → unaffected
status-firefox38.0.5:
--- → unaffected
status-firefox39:
--- → affected
status-firefox40:
--- → affected
status-firefox41:
--- → affected
status-firefox-esr31:
--- → unaffected
status-firefox-esr38:
--- → unaffected
Reporter | ||
Updated•9 years ago
|
Flags: firefox-backlog+
Comment 3•9 years ago
|
||
emtwo, if this is regressed on Beta 39, then bug 1158859 probably didn't regress it? Or maybe only made it worse if it is indeed 1158859?
Flags: needinfo?(msamuel)
Comment 5•9 years ago
|
||
From bug 1172300 attachment 8616431 [details], it looks like there is an okay amount of padding in 39. And attachment 8609589 [details] shows a smaller but not-incorrect amount of padding in 39, so there might have been an tiles-unrelated change from 38 -> 39. But I would guess bug 1158859 changed some menu sizing causing the bad croppiness in 40.
Assignee | ||
Comment 6•9 years ago
|
||
Assignee | ||
Updated•9 years ago
|
Attachment #8616755 -
Flags: review?(vdjeric)
Attachment #8616755 -
Flags: review?(msamuel)
Attachment #8616755 -
Flags: review?(mconley)
Reporter | ||
Comment 7•9 years ago
|
||
Comment on attachment 8616755 [details] [diff] [review] Change Search Settings panel is sized incorrectly on new tab page for beta/aurora I think :emtwo is the right choice here.
Attachment #8616755 -
Flags: review?(vdjeric)
Attachment #8616755 -
Flags: review?(mconley)
Comment 8•9 years ago
|
||
Comment on attachment 8616755 [details] [diff] [review] Change Search Settings panel is sized incorrectly on new tab page for beta/aurora > .newtab-customize-panel-item, > .newtab-search-panel-engine, > #newtab-search-manage { >- padding: 10px 10px 10px 25px; >+ line-height: 54px; >+ padding: 3px 10px; Are there other patches applied? m-c has different context: http://mxr.mozilla.org/mozilla-central/source/browser/base/content/newtab/newTab.css#498
Assignee | ||
Comment 9•9 years ago
|
||
Comment on attachment 8616755 [details] [diff] [review] Change Search Settings panel is sized incorrectly on new tab page for beta/aurora This patch is for beta and aurora. The patch for Bug 116764 includes the fix for the panel and is for central
Assignee | ||
Comment 10•9 years ago
|
||
Bug 1167643 ** Sorry
Assignee | ||
Comment 11•9 years ago
|
||
Comment on attachment 8616755 [details] [diff] [review] Change Search Settings panel is sized incorrectly on new tab page for beta/aurora This patch is for beta and aurora. The patch for Bug 1167643 includes the fix for the panel and is for central
Assignee | ||
Comment 12•9 years ago
|
||
Comment on attachment 8616755 [details] [diff] [review] Change Search Settings panel is sized incorrectly on new tab page for beta/aurora Try push link: https://treeherder.mozilla.org/#/jobs?repo=try&revision=e2a92053bbdd
Attachment #8616755 -
Attachment description: Change Search Settings panel is sized incorrectly on new tab page → Change Search Settings panel is sized incorrectly on new tab page for beta/aurora
Comment 13•9 years ago
|
||
Ed, the regression began in bug 1126186. Ursula, I would recommend undoing the changes made in the diff here: https://bugzilla.mozilla.org/attachment.cgi?id=8578967&action=diff We can return to "padding: 4px 24px;" only for .newtab-search-panel-engine and #newtab-search-manage and keep the current styling just for .newtab-customize-panel-item. Thanks!
Flags: needinfo?(msamuel)
Comment 14•9 years ago
|
||
Comment on attachment 8616755 [details] [diff] [review] Change Search Settings panel is sized incorrectly on new tab page for beta/aurora Review of attachment 8616755 [details] [diff] [review]: ----------------------------------------------------------------- Please refer to comment 13.
Attachment #8616755 -
Flags: review?(msamuel)
Assignee | ||
Comment 15•9 years ago
|
||
Assignee | ||
Comment 16•9 years ago
|
||
Assignee | ||
Updated•9 years ago
|
Attachment #8616755 -
Attachment is obsolete: true
Assignee | ||
Updated•9 years ago
|
Attachment #8616902 -
Flags: review?(msamuel)
Updated•9 years ago
|
Attachment #8616902 -
Flags: review?(msamuel) → review+
Reporter | ||
Updated•9 years ago
|
Assignee: nobody → ursulasarracini
Assignee | ||
Comment 17•9 years ago
|
||
Comment on attachment 8616902 [details] [diff] [review] Change Search Settings panel is sized incorrectly on new tab page for beta/aurora Approval Request Comment [Feature/regressing bug #]: Bug 1126186 [User impact if declined]: Panel UI will look very awful (see screenshot) [Describe test coverage new/current, TreeHerder]: Manual testing [Risks and why]: Low risk because it's CSS styling [String/UUID change made/needed]: None
Attachment #8616902 -
Attachment description: Change Search Settings panel is sized incorrectly on new tab page → Change Search Settings panel is sized incorrectly on new tab page for beta/aurora
Attachment #8616902 -
Flags: approval-mozilla-beta?
Attachment #8616902 -
Flags: approval-mozilla-aurora?
Updated•9 years ago
|
Iteration: --- → 41.3 - Jun 29
Points: --- → 3
Whiteboard: .?
Comment 18•9 years ago
|
||
I see that it's a minor change, but since this isn't so noticeable on beta as it is in nightly and aurora, I'd like to uplift this to 40 and let it ride the trains there.
Comment 19•9 years ago
|
||
Comment on attachment 8616902 [details] [diff] [review] Change Search Settings panel is sized incorrectly on new tab page for beta/aurora Approved for uplift to aurora. Noticeable regression for users, small css fix.
Attachment #8616902 -
Flags: approval-mozilla-beta?
Attachment #8616902 -
Flags: approval-mozilla-beta-
Attachment #8616902 -
Flags: approval-mozilla-aurora?
Attachment #8616902 -
Flags: approval-mozilla-aurora+
Reporter | ||
Comment 22•9 years ago
|
||
According to nhnt11 in bug 1173543, this doesn't need to get fixed on central as the panel itself is getting torn out. We will, however, need a fix for Aurora.
Assignee | ||
Comment 23•9 years ago
|
||
Assignee | ||
Updated•9 years ago
|
Attachment #8616902 -
Attachment is obsolete: true
Assignee | ||
Updated•9 years ago
|
Attachment #8620978 -
Attachment description: Change Search Settings panel is sized incorrectly on new tab page → Change Search Settings panel is sized incorrectly on new tab page (for aurora)
Attachment #8620978 -
Flags: review?(msamuel)
Updated•9 years ago
|
Whiteboard: .?
Comment 24•9 years ago
|
||
Comment on attachment 8620978 [details] [diff] [review] Change Search Settings panel is sized incorrectly on new tab page (for aurora) Review of attachment 8620978 [details] [diff] [review]: ----------------------------------------------------------------- ::: browser/base/content/newtab/newTab.css @@ +548,1 @@ > .newtab-search-panel-engine, We want to still keep .newtab-customize-panel-item here so that we don't change its other properties (line-height, font-size, cursor). But we need to override it's padding (see next comment) @@ -545,4 @@ > .newtab-search-panel-engine, > #newtab-search-manage { > line-height: 54px; > - padding-left: 40px; We need to keep this padding but only for .newtab-customize-panel-item
Attachment #8620978 -
Flags: review?(msamuel)
Assignee | ||
Comment 25•9 years ago
|
||
Assignee | ||
Updated•9 years ago
|
Attachment #8620978 -
Attachment is obsolete: true
Assignee | ||
Updated•9 years ago
|
Attachment #8621146 -
Attachment description: Change Search Settings panel is sized incorrectly on new tab page → Change Search Settings panel is sized incorrectly on new tab page (for aurora)
Attachment #8621146 -
Flags: review?(msamuel)
Comment 26•9 years ago
|
||
Comment on attachment 8621146 [details] [diff] [review] Change Search Settings panel is sized incorrectly on new tab page (for aurora) Review of attachment 8621146 [details] [diff] [review]: ----------------------------------------------------------------- Looks good thanks!
Attachment #8621146 -
Flags: review?(msamuel) → review+
Reporter | ||
Comment 27•9 years ago
|
||
I assume we won't need another aurora approval request, as this has just been rejigged to apply to aurora, but still fits the same conditions as in comment 17.
Assignee | ||
Updated•9 years ago
|
Attachment #8616900 -
Attachment is obsolete: true
Assignee | ||
Updated•9 years ago
|
Keywords: checkin-needed
Updated•9 years ago
|
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 40
Updated•9 years ago
|
QA Whiteboard: [good first verify]
Comment 29•9 years ago
|
||
I have reproduced this bug in Nightly 41.0a1(Build ID: 20150522030225)(UA: Mozilla/5.0 (Windows NT 6.3; WOW64; rv:41.0) Gecko/20100101 Firefox/41.0) with instruction from comment 0 bug is fixed now on latest Developer Edition 40.0a2 (Build ID: 20150623004006) Mozilla/5.0 (Windows NT 6.3; WOW64; rv:40.0) Gecko/20100101 Firefox/40.0 [Bugday-20150624]
Comment 30•9 years ago
|
||
Reproduced this bug in Nightly 41.0a1 (2015-05-22) (Build ID: 20150522030225) with instruction from comment 0 and Linux x64. Verified as fixed with latest Developer Edition 40.0a2 (2015-06-23) Build ID: 20150623004006 Mozilla/5.0 (X11; Linux x86_64; rv:40.0) Gecko/20100101 Firefox/40.0
Status: RESOLVED → VERIFIED
QA Whiteboard: [good first verify] → [good first verify][Bugday-20150624]
Whiteboard: [Bugday-20150624]
Updated•9 years ago
|
Assignee | ||
Comment 32•9 years ago
|
||
A change in padding made moved the "change search settings" text back over in aurora and beta so this bug is re-opened and I'll post a patch to fix the padding again. Changeset that caused padding change: https://hg.mozilla.org/releases/mozilla-aurora/rev/e7c0ddb5f948
Status: VERIFIED → REOPENED
Resolution: FIXED → ---
Assignee | ||
Comment 33•9 years ago
|
||
Assignee | ||
Updated•9 years ago
|
Attachment #8621146 -
Attachment is obsolete: true
Assignee | ||
Updated•9 years ago
|
Attachment #8645753 -
Flags: review?(msamuel)
Comment 34•9 years ago
|
||
Comment on attachment 8645753 [details] [diff] [review] Change Search Settings panel is sized incorrectly on new tab page for aurora Review of attachment 8645753 [details] [diff] [review]: ----------------------------------------------------------------- ::: browser/base/content/newtab/newTab.css @@ +559,5 @@ > max-width: 300px; > } > > +.newtab-customize-panel-item { > + padding-left: 40px; We need this to be -moz-padding-start so rtl locales get the correct padding too.
Attachment #8645753 -
Flags: review?(msamuel) → review+
Assignee | ||
Comment 35•9 years ago
|
||
Assignee | ||
Updated•9 years ago
|
Attachment #8645753 -
Attachment is obsolete: true
Assignee | ||
Updated•9 years ago
|
Attachment #8645999 -
Flags: review+
Assignee | ||
Updated•9 years ago
|
Keywords: checkin-needed
Assignee | ||
Updated•9 years ago
|
Assignee | ||
Updated•9 years ago
|
Target Milestone: Firefox 40 → Firefox 41
Reporter | ||
Comment 36•9 years ago
|
||
I think this is going to need beta uplift approval set on it in order to get checked in.
Keywords: checkin-needed
Reporter | ||
Comment 37•9 years ago
|
||
Hey Ursula - can you request beta approval on this when you get a chance? I think that's the only place where this patch needs to land.
Flags: needinfo?(ursulasarracini)
Assignee | ||
Comment 38•9 years ago
|
||
Comment on attachment 8645999 [details] [diff] [review] Change Search Settings panel is sized incorrectly on new tab page for aurora Approval Request Comment [Feature/regressing bug #]: [User impact if declined]: [Describe test coverage new/current, TreeHerder]: [Risks and why]: [String/UUID change made/needed]:
Flags: needinfo?(ursulasarracini)
Attachment #8645999 -
Flags: approval-mozilla-beta?
Assignee | ||
Comment 39•9 years ago
|
||
Approval Request Comment [Feature/regressing bug #]: Bug 1172987 [User impact if declined]: UI will not look nice (see original screenshot) [Describe test coverage new/current, TreeHerder]: Manual testing [Risks and why]: Low risk since it's just CSS [String/UUID change made/needed]: None
Comment on attachment 8645999 [details] [diff] [review] Change Search Settings panel is sized incorrectly on new tab page for aurora Approved, the mis-alignment of text looks sloppy and is a regression. Let's uplift to Beta, seems safe and the fix is straightforward too.
Attachment #8645999 -
Flags: approval-mozilla-beta? → approval-mozilla-beta+
Requesting QE team to verify this is fixed on all the current releases, FF43, FF42, FF41. Thanks!
Flags: qe-verify+
Comment 42•9 years ago
|
||
Ursula, next time, please reopen a new bug. Here, it is hard to understand which versions is affected or not and bugs are cheap. Mike, I think Ursula is in PTO, could you help us by updating the tracking flags? Is it normal that the attachment 8645999 [details] [diff] [review] has no uplift requests to aurora? Thanks
Reporter | ||
Comment 44•9 years ago
|
||
(In reply to Sylvestre Ledru [:sylvestre] from comment #42) > Mike, I think Ursula is in PTO, could you help us by updating the tracking > flags? Is it normal that the attachment 8645999 [details] [diff] [review] > has no uplift requests to aurora? Thanks Yeah, I can explain what's going on here. The original bug report affected Nightly 41, Aurora 40 and Beta 39. In comment 18, lizzard describes how since the problem wasn't as visibly bad on 39 that we should land the patch on 40 and let it ride the trains. Uplifts then occurred, and 39 was out the door. Then, on Nightly 42, bug 1171344 landed, which eliminates the panel that this bug refers to. Then in bug 1172987, a patch was uplifted to Aurora 41 which re-opened this bug (I agree in hindsight that we should have opened a new bug). Then uplifts occurred. So the problem affects Beta 41, but does not affect Aurora 42 or Nightly 43, since the panel has been eliminated in the latter two releases.
Updated•9 years ago
|
QA Whiteboard: [good first verify][Bugday-20150624] → [Bugday-20150624]
Assignee | ||
Comment 45•9 years ago
|
||
Closing this bug based on the updated flags and the fact that this landed in beta, which seems to be the only version that this was affecting
Status: REOPENED → RESOLVED
Closed: 9 years ago → 9 years ago
Resolution: --- → WONTFIX
You need to log in
before you can comment on or make changes to this bug.
Description
•