The 'Search' placeholder should appear immediately on about:home and about:newtab

VERIFIED FIXED in Firefox 56

Status

()

Firefox
Search
P1
normal
VERIFIED FIXED
2 months ago
a month ago

People

(Reporter: florian, Assigned: florian)

Tracking

(Blocks: 1 bug)

Trunk
Firefox 56
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox56 verified)

Details

(Whiteboard: [photon-performance])

Attachments

(1 attachment, 1 obsolete attachment)

(Assignee)

Description

2 months ago
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

2 months ago
Created attachment 8876331 [details] [diff] [review]
The 'Search' placeholder should appear immediately on about:home and about:newtab, r=
Attachment #8876331 - Flags: review?(nhnt11)
(Assignee)

Updated

2 months ago
Assignee: nobody → florian
Status: NEW → ASSIGNED
(Assignee)

Comment 2

2 months 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

2 months ago
Created attachment 8876599 [details] [diff] [review]
Patch v2
Attachment #8876599 - Flags: review?(nhnt11)
(Assignee)

Updated

2 months ago
Attachment #8876331 - Attachment is obsolete: true
Comment on attachment 8876599 [details] [diff] [review]
Patch v2

Review of attachment 8876599 [details] [diff] [review]:
-----------------------------------------------------------------

LGTM, thanks!
Attachment #8876599 - Flags: review?(nhnt11) → review+

Comment 5

2 months ago
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

2 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/24549b6535ec
Status: ASSIGNED → RESOLVED
Last Resolved: 2 months ago
status-firefox56: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 56

Updated

2 months ago
Iteration: --- → 56.1 - Jun 26
Flags: qe-verify?
Priority: -- → P1
Whiteboard: [photon-performance]

Comment 7

2 months ago
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

2 months ago
The issue is no longer reproducible on Firefox 56.0a1. Tests were performed under Windows 10x64.
Build ID: 20170628030206
[bugday-20170628]
status-firefox56: fixed → verified
(Assignee)

Comment 9

a month ago
Verified per comment 8.
Status: RESOLVED → VERIFIED

Updated

a month ago
Flags: qe-verify?
You need to log in before you can comment on or make changes to this bug.