Closed Bug 1378330 Opened 2 years ago Closed 2 years ago

about:addons search field lost its submit button

Categories

(Toolkit :: Themes, defect, P1)

defect

Tracking

()

VERIFIED FIXED
mozilla56
Tracking Status
firefox-esr52 --- unaffected
firefox54 --- unaffected
firefox55 + fixed
firefox56 + verified

People

(Reporter: dao, Assigned: rickychien)

References

(Blocks 1 open bug)

Details

(Keywords: regression, Whiteboard: [photon-preference])

Attachments

(2 files)

[Tracking Requested - why for this release]:

Bug 1360491 broke this on Windows and Linux, Mac was broken before. Ricky, can you take a look?
Flags: needinfo?(rchien)
Thanks for reporting that issue Dao. I'll take a look.
Assignee: nobody → rchien
Status: NEW → ASSIGNED
Flags: needinfo?(rchien)
Flags: qe-verify+
Priority: -- → P1
QA Contact: hani.yacoub
Whiteboard: [photon-preference]
Target Milestone: --- → mozilla56
Tracked since it's a recent regression in 55, 56
Blocks: 1357285
This is the file revisions history for toolkit/themes/osx/global/textbox.css. https://hg.mozilla.org/mozilla-central/log/tip/toolkit/themes/osx/global/textbox.css?revcount=30

After scanning all revisions for toolkit/themes/osx/global/textbox.css, I cannot find out any evidence of CSS rule which sets list-style-image for `.textbox-search-icon` like on Windows [1] and Linux [2], and in consequence I suspect that by design the macOS is the only platform that doesn't display the submit button.

[1] http://searchfox.org/mozilla-central/source/toolkit/themes/windows/global/textbox.css#77
[2] http://searchfox.org/mozilla-central/source/toolkit/themes/linux/global/textbox.css#71
Patch has attached and verified on macOS and Windows as well.

Dao, can you review it kindly for me?  Thank you :)
Comment on attachment 8884209 [details]
Bug 1378330 - Fix the disappeared submit button in search field on Win & Linux for beta

https://reviewboard.mozilla.org/r/155166/#review160208

::: toolkit/themes/shared/in-content/common.inc.css
(Diff revision 1)
> -}
> -
> -xul|textbox[type="search"] > .textbox-input-box > .textbox-search-sign {
> -  list-style-image: url(chrome://global/skin/icons/search-textbox.svg);
> -  margin-inline-end: 5px;
> -}

You should just use :not([searchbutton]) in these two rules.
Attachment #8884209 - Flags: review?(dao+bmo) → review-
Comment on attachment 8884209 [details]
Bug 1378330 - Fix the disappeared submit button in search field on Win & Linux for beta

https://reviewboard.mozilla.org/r/155166/#review160214

::: browser/themes/shared/incontentprefs/preferences.inc.css:684
(Diff revision 3)
> +}
> +
> +textbox:not([searchbutton]) > .textbox-input-box > .textbox-search-sign {
> +  list-style-image: url(chrome://global/skin/icons/search-textbox.svg);
> +  margin-inline-end: 5px;
> +}

[type="search"] should not be removed from these selectors.

Why did you move this from common.inc.css?
Attachment #8884209 - Flags: review?(dao+bmo) → review-
Comment on attachment 8884209 [details]
Bug 1378330 - Fix the disappeared submit button in search field on Win & Linux for beta

https://reviewboard.mozilla.org/r/155166/#review160228

::: browser/themes/shared/incontentprefs/preferences.inc.css:679
(Diff revision 4)
>    transform: scaleX(-1);
>  }
> +
> +textbox[type="search"]:not([searchbutton]) > .textbox-input-box > .textbox-search-icons > .textbox-search-icon {
> +  visibility: hidden;
> +}

Again, why not leave this in common.inc.css?
Attachment #8884209 - Flags: review?(dao+bmo) → review-
Comment on attachment 8884209 [details]
Bug 1378330 - Fix the disappeared submit button in search field on Win & Linux for beta

https://reviewboard.mozilla.org/r/155166/#review160228

> Again, why not leave this in common.inc.css?

In order to unhide the submit button for about:addons, we should move all in-content/common.inc.css changes to preferences.inc.css.
Ah, sorry. I think I know why you ask me to add :not([searchbutton]). It's the easiest way to solve this issue without moving textbox rule to preferences.inc.css.
Comment on attachment 8884209 [details]
Bug 1378330 - Fix the disappeared submit button in search field on Win & Linux for beta

https://reviewboard.mozilla.org/r/155166/#review160262
Attachment #8884209 - Flags: review?(dao+bmo) → review+
Pushed by rchien@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/cf06740a256b
Fix the disappeared submit button in search field on Win & Linux r=dao
https://hg.mozilla.org/mozilla-central/rev/cf06740a256b
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Build ID: 20170709030204
User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:56.0) Gecko/20100101 Firefox/56.0

Verified as fixed on Firefox Nightly 56.0a1 on Windows 10 x 64 and Ubuntu 16.04 x64.
Status: RESOLVED → VERIFIED
Hi Ricky, Dao, should we uplift this fix (if it is low risk) to Beta55? If yes, please nominate a patch.
Flags: needinfo?(rchien)
Flags: needinfo?(dao+bmo)
Comment on attachment 8884209 [details]
Bug 1378330 - Fix the disappeared submit button in search field on Win & Linux for beta

Approval Request Comment
[Feature/Bug causing the regression]: Bug 1360491
[User impact if declined]: UI inconsistency
[Is this code covered by automated tests?]: no
[Has the fix been verified in Nightly?]: not yet
[Needs manual test from QE? If yes, steps to reproduce]: no
[List of other uplifts needed for the feature/fix]: no
[Is the change risky?]: minor
[Why is the change risky/not risky?]: only CSS changes for fixing UI inconsistency
[String changes made/needed]: no
Flags: needinfo?(rchien)
Flags: needinfo?(dao+bmo)
Attachment #8884209 - Flags: approval-mozilla-beta?
Comment on attachment 8884209 [details]
Bug 1378330 - Fix the disappeared submit button in search field on Win & Linux for beta

css fix for beta55+
Attachment #8884209 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
needs a rebased patch

grafting 428342:cf06740a256b "Bug 1378330 - Fix the disappeared submit button in search field on Win & Linux r=dao"
^[[Cmerging toolkit/themes/shared/in-content/common.inc.css
warning: conflicts while merging toolkit/themes/shared/in-content/common.inc.css! (edit, then use 'hg resolve --mark')
abort: unresolved conflicts, can't continue
(use 'hg resolve' and 'hg graft --continue')
Flags: needinfo?(rchien)
Attached patch beta.diffSplinter Review
I have no idea what's the best way to upload a new patch for beta through mozreview.

I uploaded the beta tweaked version so that can fit into beta branch. Please use beta.diff for 55 upliftings. Thanks
Flags: needinfo?(rchien)
Please see comment 24.
Flags: needinfo?(cbook)
(In reply to Ricky Chien [:rickychien] from comment #24)
> Created attachment 8885556 [details] [diff] [review]
> beta.diff
> 
> I have no idea what's the best way to upload a new patch for beta through
> mozreview.
> 
> I uploaded the beta tweaked version so that can fit into beta branch. Please
> use beta.diff for 55 upliftings. Thanks

no worries, all done :)
Flags: needinfo?(cbook)
https://hg.mozilla.org/projects/jamun/rev/3e01f819ec53b907d3b870ec6946a5d2f51b3b20
Bug 1378330 - Fix the disappeared submit button in search field on Win & Linux for beta. r=dao a=jcristau
You need to log in before you can comment on or make changes to this bug.