Closed Bug 1324171 Opened 4 years ago Closed 4 years ago

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

Categories

(Firefox :: Preferences, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
Firefox 53
Tracking Status
firefox52 --- unaffected
firefox53 --- fixed

People

(Reporter: jaws, Assigned: leftysolara, Mentored)

References

(Blocks 1 open bug)

Details

Attachments

(2 files)

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
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
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
Status: NEW → ASSIGNED
Flags: needinfo?(jaws)
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.
(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 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 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+
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
https://hg.mozilla.org/mozilla-central/rev/9d13ec3eac00
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 53
QA Whiteboard: [good first verify]
 [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
This was never uplifted to Firefox 52 so it can't be verified there.
You need to log in before you can comment on or make changes to this bug.