Closed Bug 1361244 Opened 7 years ago Closed 7 years ago

[Form Autofill] Add a new pseudo class for preview which highlight and change text color of form elements.

Categories

(Toolkit :: Form Manager, enhancement, P3)

Product:
Toolkit
Component:
Form Manager
Version:
52 Branch
Type:
enhancement

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla55
Tracking Flags:
Tracking Status
firefox55 --- fixed

People

(Reporter: ralin, Assigned: ralin)

References

Details

(Whiteboard: [form autofill:M3])

Attachments

(1 file)

Bug 1361244 - Add an internal -moz-autofill-preview pseudo class for styling preview fields.
59 bytes, text/x-review-board-request
heycam
: review+
Details 
After bug 1355438 landed, we got an elegant way to toggle highlight for auto-filled elements. That is close to but doesn't meet the requirement of preview styling which not only highlight but also changing text color. We would need a new pseudo class for preview, and maybe use it like: 

:-moz-autofill:-moz-autofill-preview {
 color: GrayText;
}
Assignee: nobody → ralin
Status: NEW → ASSIGNED
Hi Cameron,


Sorry to bother you again :P

Besides -moz-autofill implemented by Luke(Bug 1355438), we need a extra state for preview.

fields style in two states:
- preview: background highlight + gray text
- auto-filled: background highlight

I don't know if there's any concern about having too many MANUALLY_MANAGED_STATES, but it would be more clear if we can declare another separate pseudo class for preview. Though we need to reuse highlight style right now, perhaps, a advantage I can think of is if someday :-moz-autofill could be styled by web author, we won't unexpectedly change preview color then.


Could you help me review this patch? Thanks
Comment on attachment 8864390 [details]
Bug 1361244 - Add an internal -moz-autofill-preview pseudo class for styling preview fields.

https://reviewboard.mozilla.org/r/136074/#review139448

Just to confirm: by the time we want to ship this feature, we'll want to have a different mechanism to affect the styling of the <input> elements, since we don't want the page to be able to see the filter/color properties here, right?

::: layout/style/res/forms.css:1207
(Diff revision 2)
>  :-moz-autofill {
>    filter: grayscale(21%) brightness(88%) contrast(161%) invert(10%) sepia(40%) saturate(206%);
>  }
> +:-moz-autofill-preview {
> +  filter: grayscale(21%) brightness(88%) contrast(161%) invert(10%) sepia(40%) saturate(206%);
> +  color: GrayText;
> +}

Will an element ever match both of these pseudo-classes, or just one?  If it's only ever one, then can you combine the rules like this to avoid repetition:

  :-moz-autofill, :-moz-autofill-preview {
    filter: ...;
  }
  :-moz-autofill-preview {
    color: GrayText;
  }

If it's both, then you can remove the |filter| declaration from the second rule.
Attachment #8864390 - Flags: review?(cam) → review+
(In reply to Cameron McCormack (:heycam) from comment #4)
> Comment on attachment 8864390 [details]
> Bug 1361244 - Add an internal -moz-autofill-preview pseudo class for styling
> preview fields.
> 
> https://reviewboard.mozilla.org/r/136074/#review139448
> 
> Just to confirm: by the time we want to ship this feature, we'll want to
> have a different mechanism to affect the styling of the <input> elements,
> since we don't want the page to be able to see the filter/color properties
> here, right?
> 

Yes :)

> ::: layout/style/res/forms.css:1207
> (Diff revision 2)
> >  :-moz-autofill {
> >    filter: grayscale(21%) brightness(88%) contrast(161%) invert(10%) sepia(40%) saturate(206%);
> >  }
> > +:-moz-autofill-preview {
> > +  filter: grayscale(21%) brightness(88%) contrast(161%) invert(10%) sepia(40%) saturate(206%);
> > +  color: GrayText;
> > +}
> 
> Will an element ever match both of these pseudo-classes, or just one?  If
> it's only ever one, then can you combine the rules like this to avoid
> repetition:
> 
>   :-moz-autofill, :-moz-autofill-preview {
>     filter: ...;
>   }
>   :-moz-autofill-preview {
>     color: GrayText;
>   }
> 
> If it's both, then you can remove the |filter| declaration from the second
> rule.
The state transition would be :-moz-autofill-preview ---> :moz-autofill, so only match one at the same time. I've removed the duplicate rules according to your suggestion. 

Thanks!
Keywords: checkin-needed
Pushed by mozilla@noorenberghe.ca:
https://hg.mozilla.org/integration/mozilla-inbound/rev/75d9703cd375
Add an internal -moz-autofill-preview pseudo class for styling preview fields. r=heycam
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/75d9703cd375
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
status-firefox55: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
Is there a bug to make this work for stylo?
Flags: needinfo?(cam)
No.  Filed bug 1364354.
Flags: needinfo?(cam)
Flags: qe-verify-
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: