1411236 - Conflicting sidebar search bar padding rules on Windows

Status: VERIFIED FIXED

(Firefox :: Theme, defect, P1)

Description

[(inactive) Nihanth Subramanya [:nhnt11]]

:(

Uploading a patch, though it's probably too late to uplift to 57.

Comment 1

[(inactive) Nihanth Subramanya [:nhnt11]]

Attachment: Bug 1411236 - Conflicting sidebar search bar padding rules on Windows.

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

Comment 2

[(inactive) Nihanth Subramanya [:nhnt11]]

I didn't write much of a bug description because the title gives most of the information, but here are some details:

We increased the padding around the searchbar in bug 1400165 but I missed the fact that Windows already had a rule for this, and added the new rule near the top of the stylesheet. The older rule was further down, so the new rule gets overridden.

Comment 3

[Dão Gottwald [:dao]]

Comment on attachment 8921434 [details]
Bug 1411236 - Conflicting sidebar search bar padding rules on Windows.

https://reviewboard.mozilla.org/r/192462/#review197614

Comment 4

[(inactive) Nihanth Subramanya [:nhnt11]]

Comment on attachment 8921434 [details]
Bug 1411236 - Conflicting sidebar search bar padding rules on Windows.

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

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/fc51ba0a7e8d
Conflicting sidebar search bar padding rules on Windows. r=dao

Comment 6

[(inactive) Nihanth Subramanya [:nhnt11]]

Comment on attachment 8921434 [details]
Bug 1411236 - Conflicting sidebar search bar padding rules on Windows.

Approval Request Comment
[Feature/Bug causing the regression]: photon-visual polish. Not a regression, but makes sure the change from bug 1400165 actually takes effect on Windows.
[User impact if declined]: Search bar in sidebar is a little cramped on Windows; less spacing around it. Inconsistent with MacOS and Linux.
[Is this code covered by automated tests?]: no
[Has the fix been verified in Nightly?]: no
[Needs manual test from QE? If yes, steps to reproduce]:
[List of other uplifts needed for the feature/fix]: bug 1400165 was already uplifted.
[Is the change risky?]: no
[Why is the change risky/not risky?]: We had one CSS rule applying 8px of padding and one applying 4px to the same element. The patch un-conflicts this rule
[String changes made/needed]: none

I realize it's pretty late but I thought I'd request uplift anyway and see what happens :)

Comment 7

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/fc51ba0a7e8d

Comment 8

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Comment on attachment 8921434 [details]
Bug 1411236 - Conflicting sidebar search bar padding rules on Windows.

This is a follow up fix to bug 1400165 was triaged as a P1, Beta57+

Comment 9

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/44dfa8d7a234

Comment 10

[ovidiu boca[:Ovidiu]]

Hi Nihanth, can you please help us here and tell us how should we verify this bug? Thanks

Comment 11

[Valentina Claudia Ona]

I verified this issue using Nightly 58.0a1 on Windows 7 x64 and Windows 10 x64 with Build ID 20171106100122.
I will mark this as verified fixed.

Comment 12

[Timea Zsoldos [:zstimi/tzsoldos], Desktop QA]

I have reproduced this issue using Firefox nightly build (2017.10.24) on Win 8.1 x64.
I can confirm this issue is fixed, I verified using Firefox 57.0b14 on Win 8.1 x64.