Closed
Bug 1371860
Opened 7 years ago
Closed 7 years ago
The 'Search' placeholder should appear immediately on about:home and about:newtab
Categories
(Firefox :: Search, enhancement, P1)
Firefox
Search
Tracking
()
Tracking | Status | |
---|---|---|
firefox56 | --- | verified |
People
(Reporter: florian, Assigned: florian)
References
(Blocks 1 open bug)
Details
(Whiteboard: [photon-performance])
Attachments
(1 file, 1 obsolete file)
10.40 KB,
patch
|
nhnt11
:
review+
|
Details | Diff | Splinter Review |
Currently the 'Search' placeholder is the last thing that's painted on about:home. It should be painted immediately with the rest of the page. I think the delay is due to passing the strings from the .properties file to the DOM using custom events in contentSearchUI.js (dispatchEvent(new CustomEvent("ContentSearchClient"...). Getting the string from a dtd file at the same time as we get the aria label seems simpler/faster.
Assignee | ||
Comment 1•7 years ago
|
||
Attachment #8876331 -
Flags: review?(nhnt11)
Assignee | ||
Updated•7 years ago
|
Assignee: nobody → florian
Status: NEW → ASSIGNED
Assignee | ||
Comment 2•7 years ago
|
||
Comment on attachment 8876331 [details] [diff] [review] The 'Search' placeholder should appear immediately on about:home and about:newtab, r= Review of attachment 8876331 [details] [diff] [review]: ----------------------------------------------------------------- ::: browser/locales/en-US/chrome/browser/browser.dtd @@ +468,5 @@ > This is set as the aria-label attribute for the search input box in the > in-content search UI, to be used by screen readers. --> > <!ENTITY contentSearchInput.label "Search query"> > +<!-- LOCALIZATION NOTE (contentSearchInput.placeholder): > + This string should match searchPlaceholder from search.properties --> Actually, instead of this comment, I think we should move the searchbar's placeholder to browser.dtd too. And while we are at it, we could remove the http://searchfox.org/mozilla-central/source/browser/locales/en-US/chrome/browser/searchbar.dtd file which contains only one string used by the searchbar (cmd_engineManager.label is a leftover from the previous search UI).
Attachment #8876331 -
Flags: review?(nhnt11)
Assignee | ||
Comment 3•7 years ago
|
||
Attachment #8876599 -
Flags: review?(nhnt11)
Assignee | ||
Updated•7 years ago
|
Attachment #8876331 -
Attachment is obsolete: true
Comment 4•7 years ago
|
||
Comment on attachment 8876599 [details] [diff] [review] Patch v2 Review of attachment 8876599 [details] [diff] [review]: ----------------------------------------------------------------- LGTM, thanks!
Attachment #8876599 -
Flags: review?(nhnt11) → review+
Pushed by florian@queze.net: https://hg.mozilla.org/integration/mozilla-inbound/rev/24549b6535ec The 'Search' placeholder should appear immediately on about:home and about:newtab, r=nhnt11.
Comment 6•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/24549b6535ec
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
status-firefox56:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 56
Updated•7 years ago
|
Iteration: --- → 56.1 - Jun 26
Flags: qe-verify?
Priority: -- → P1
Whiteboard: [photon-performance]
I have reproduced this bug with Nightly 55.0a1 (2017-06-09) on Windows 8.1 (64 bit). This bug's fix is verified on Latest Nightly 56.0a1. Build ID : 20170615030208 User Agent : Mozilla/5.0 (Windows NT 6.3; WOW64; rv:56.0) Gecko/20100101 Firefox/56.0
QA Whiteboard: [bugday-20170614]
Comment 8•7 years ago
|
||
The issue is no longer reproducible on Firefox 56.0a1. Tests were performed under Windows 10x64. Build ID: 20170628030206 [bugday-20170628]
Updated•7 years ago
|
Flags: qe-verify?
You need to log in
before you can comment on or make changes to this bug.
Description
•