Search fields should always have "Search" as the placeholder text instead of appearing in-front of the Search field

RESOLVED FIXED in Firefox 53

Status

()

Firefox
Preferences
RESOLVED FIXED
6 months ago
3 months ago

People

(Reporter: jaws, Assigned: Jalen Adams, Mentored)

Tracking

(Blocks: 1 bug)

Trunk
Firefox 53
Points:
---

Firefox Tracking Flags

(firefox52 unaffected, firefox53 fixed)

Details

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(2 attachments)

From slide 8 and 9 of https://bugzilla.mozilla.org/attachment.cgi?id=8819509

Updated

6 months ago
Summary: Search fields should always have "Search" as the placeholder text instead of appearing in-front of the Search field → Search fields should always have "Search" as a label instead of a placeholder in the field

Comment 1

6 months ago
unhelpful
whoops, I misread all of it :(
Summary: Search fields should always have "Search" as a label instead of a placeholder in the field → Search fields should always have "Search" as the placeholder text instead of appearing in-front of the Search field
(Assignee)

Comment 2

6 months ago
I'm willing to take this bug if no one else is working on it already.
Flags: needinfo?(jaws)
Cool, it's yours! Let me know if you have questions.
Assignee: nobody → leftysolara
Mentor: jaws@mozilla.com
Status: NEW → ASSIGNED
Flags: needinfo?(jaws)
Comment hidden (mozreview-request)
(Reporter)

Comment 5

6 months ago
mozreview-review
Comment on attachment 8820451 [details]
Bug 1324171 - Use 'Search' as placeholder text instead of as a label in-front of search fields.

https://reviewboard.mozilla.org/r/99952/#review100712

::: browser/components/preferences/cookies.xul:38
(Diff revision 1)
>      <hbox align="center">
> -      <label accesskey="&filter.accesskey;" control="filter">&filter.label;</label>
>        <textbox type="search" id="filter" flex="1"
>                 aria-controls="cookiesList"
> -               oncommand="gCookiesWindow.filter();"/>
> +               oncommand="gCookiesWindow.filter();"
> +               placeholder="Search"/>

We need to use the localizable strings here so that the placeholder text matches the language of the application.

This should have placeholder="&filter.label;" and also accesskey="&filter.accesskey;".

The same should be done for passwordManager.xul.
Attachment #8820451 - Flags: review?(jaws) → review-
You can test that the accesskey works by pressing Shift+Alt+accesskey when the field isn't focused. Pressing the combination should move focus to the field.

Previously the accesskey was visible to users by showing the underline on the accesskey character. This won't be visible anymore now that we have switched to the placeholder text, but we shouldn't break the functionality for users that are used to this working.
(Assignee)

Comment 7

6 months ago
(In reply to Jared Wein [:jaws] (please needinfo? me) from comment #5)
> We need to use the localizable strings here so that the placeholder text
> matches the language of the application.

Thanks for the tip. I'll try to keep things like that in mind in the future.

I removed the colon from the labels to match the example in the slides. Should the entities' keys be modified so all the locales see the change (like in Bug 1324173)?
Flags: needinfo?(jaws)
(In reply to Jalen Adams from comment #7)
> I removed the colon from the labels to match the example in the slides.
> Should the entities' keys be modified so all the locales see the change
> (like in Bug 1324173)?

Yeah, good call. We should update the entities' keys. We wouldn't want some of the locales to still have "Search:" as the placeholder.
Flags: needinfo?(jaws)
Comment hidden (mozreview-request)
(Reporter)

Comment 10

6 months ago
mozreview-review
Comment on attachment 8820451 [details]
Bug 1324171 - Use 'Search' as placeholder text instead of as a label in-front of search fields.

https://reviewboard.mozilla.org/r/99952/#review101056

Perfect!
Attachment #8820451 - Flags: review?(jaws) → review+
It looks like autoland on this failed. It might need to be rebased.
Something got screwed up with the revisions of the patch on this bug. The first revision has the labels being removed and "Search" hardcoded as the placeholder. The second revision shows "Search" being changed to use the locale string, but the history doesn't connect the two.

I've fixed it on my machine and am building with it right now. Once the build is done, if it looks good I'll get it landed.
Comment hidden (mozreview-request)
(Reporter)

Comment 14

6 months ago
mozreview-review
Comment on attachment 8821205 [details]
Bug 1324171 - Use 'Search' as placeholder text instead of as a label in-front of search fields.

https://reviewboard.mozilla.org/r/100542/#review101084

Fixed up the patch and tested that it worked.
Attachment #8821205 - Flags: review?(jaws) → review+

Comment 15

6 months ago
Pushed by jwein@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/9d13ec3eac00
Use 'Search' as placeholder text instead of as a label in-front of search fields. r=jaws

Comment 16

6 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/9d13ec3eac00
Status: ASSIGNED → RESOLVED
Last Resolved: 6 months ago
status-firefox53: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 53
QA Whiteboard: [good first verify]

Comment 17

3 months ago
 [testday-20170317] 
Bug 1324171 - Use 'Search' as placeholder text instead of as a label in-front of search fields is verified.
BROWSER: Firefox Beta 52.0
OS: Windows 10.0
status-firefox52: --- → verified
This was never uplifted to Firefox 52 so it can't be verified there.
status-firefox52: verified → unaffected
You need to log in before you can comment on or make changes to this bug.