Group item title field should use HTML5 placeholder

VERIFIED FIXED in Firefox 4.0b10

Status

P4
enhancement
VERIFIED FIXED
8 years ago
3 years ago

People

(Reporter: mitcho, Assigned: mitcho)

Tracking

unspecified
Firefox 4.0b10

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments, 3 obsolete attachments)

Right now we have some special case code in order to display the "Name this tab group..." text and icon on unnamed groups. I believe all of this JS is unnecessary and it would all be cleaner if we used a placeholder attribute on the title input field and then styled it with :-moz-placeholder .

This most likely would require a litmus test as I'm not sure how an automated test for this would be written.
Posted video Video of patch v1
Looks like this means we would have to get rid of the behavior whereby the pencil icon stays when you enter a name for the first time, and gracefully disappears later... but I think making this all CSS is enough benefit. Aza, thoughts?
Assignee: nobody → mitcho
Status: NEW → ASSIGNED
Attachment #497869 - Flags: ui-review?(aza)
Posted patch Patch v1 (obsolete) — Splinter Review
Quick first patch... note all the JS we can get rid of!

Note: this patch only includes necessary CSS changes for Mac, not yet other themes.
Attachment #497870 - Flags: feedback?(ian)
Comment on attachment 497870 [details] [diff] [review]
Patch v1

Looks great!
Attachment #497870 - Flags: feedback?(ian) → feedback+
Comment on attachment 497870 [details] [diff] [review]
Patch v1

>-  -moz-margin-begin: 3px;
>+  -moz-margin-start: 3px;

Thanks for catching this, by the way! While you're in here, please fix the other one (title-shield)... for some reason it's only in the Mac theme.
Attachment #497869 - Attachment is patch: false
Attachment #497869 - Attachment mime type: text/plain → video/ogg
Posted patch Patch v1.1 (obsolete) — Splinter Review
- Applied the same treatment to winstripe and gnomestripe.
- Took care of that one last -moz-margin-begin
Attachment #497870 - Attachment is obsolete: true
Attachment #499533 - Flags: review?(ian)

Updated

8 years ago
Attachment #497869 - Flags: ui-review?(aza) → ui-review+
Comment on attachment 499533 [details] [diff] [review]
Patch v1.1

Lovely
Attachment #499533 - Flags: review?(ian) → review+
Comment on attachment 499533 [details] [diff] [review]
Patch v1.1

>-.title-container:hover input.name {
>+.title-container:hover input.name, .title-container input.name:focus {

Break the line after the comma.
Posted patch Patch v1.2 (obsolete) — Splinter Review
Thanks Dāo. Fixed.
Attachment #499533 - Attachment is obsolete: true
Attachment #501073 - Flags: approval2.0?
Pushed patch v1.2 to try today. Passed.
Comment on attachment 501073 [details] [diff] [review]
Patch v1.2

a=beltzner
Attachment #501073 - Flags: approval2.0? → approval2.0+
Pushed to try as well, just to be safe.
Attachment #501073 - Attachment is obsolete: true
Passed try.
Keywords: checkin-needed
http://hg.mozilla.org/mozilla-central/rev/f9537a28ce19
Status: ASSIGNED → RESOLVED
Last Resolved: 8 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 4.0b10
verified with minefield build of 20110217
Status: RESOLVED → VERIFIED

Updated

8 years ago
Duplicate of this bug: 597792
Product: Firefox → Firefox Graveyard
You need to log in before you can comment on or make changes to this bug.