Closed
Bug 1069012
Opened 10 years ago
Closed 8 years ago
unprefix :placeholder-shown pseudo-class and ::placeholder pseudo-element
Categories
(Core :: Layout: Form Controls, defect)
Core
Layout: Form Controls
Tracking
()
RESOLVED
FIXED
mozilla51
Tracking | Status | |
---|---|---|
firefox51 | --- | fixed |
People
(Reporter: dbaron, Assigned: wisniewskit)
References
(Blocks 2 open bugs)
Details
(Keywords: dev-doc-complete, site-compat, Whiteboard: [parity-webkit])
Attachments
(1 file, 2 obsolete files)
61.57 KB,
patch
|
Details | Diff | Splinter Review |
Maybe not yet, but at some point we should unprefix the :placeholder-shown pseudo-class and ::placeholder pseudo-element.
We had code for the pseudo-class, but it was removed in bug 737786, which added support for the pseudo-element, which we currently implement prefixed.
The names were agreed by the CSSWG in http://krijnhoetmer.nl/irc-logs/css/20130206#l-654 . Only the pseudo-class is in a spec http://dev.w3.org/csswg/selectors/#placeholder partly because there doesn't appear to be an active spec approprite for the pseudo-element.
http://caniuse.com/#feat=css-placeholder is somewhat misleading.
Updated•10 years ago
|
Keywords: dev-doc-needed
Reporter | ||
Comment 2•10 years ago
|
||
The CSS working group resolved on these names after a long discussion; I wouldn't reopen that discussion without a very good reason.
@percyley, just checked, is not jet supported on Chrome, not even on Chrome canary 49.0.2593.0.
Released in Chrome 47 stable with tons of bugs https://code.google.com/p/chromium/issues/detail?id=451120#c13
Comment 6•9 years ago
|
||
::placeholder has been unprefixed in Safari Tech Preview 7.
See https://webkit.org/blog/6640/release-notes-for-safari-technology-preview-release-7/
> * Added the unprefixed version of the pseudo element ::placeholder (r202066)
Whiteboard: [parity-webkit]
Comment 7•9 years ago
|
||
(In reply to David Baron :dbaron: ⌚️UTC-7 from comment #0)
> Only the pseudo-class is in a spec [...] partly because
> there doesn't appear to be an active spec appropriate for the pseudo-element.
::placeholder is currently part of Pseudo-Elements Level 4:
https://www.w3.org/TR/css-pseudo-4/#placeholder-pseudo
Assignee | ||
Comment 8•8 years ago
|
||
Since I'm not 100% sure if we'd need to keep both the prefixed and unprefixed versions for now (which is proving trickier), I figured I'd first offer a version which just removes the unprefixed version. A try run is fine: https://treeherder.mozilla.org/#/jobs?repo=try&revision=1ba7eb550f0c&selectedJob=26870596
I'll work on a version that keeps the prefixed unprefixed versions though, since I'm guessing there is no telemetry on whether it's safe to just support the unprefixed version.
Assignee | ||
Comment 9•8 years ago
|
||
And here's a patch with both the prefixed and unprefixing versions. A try run only shows unrelated stuff: https://treeherder.mozilla.org/#/jobs?repo=try&revision=07f7958d9c5a
Attachment #8787829 -
Attachment is obsolete: true
Attachment #8787829 -
Flags: review?(bzbarsky)
Attachment #8787919 -
Flags: review?(bzbarsky)
![]() |
||
Comment 10•8 years ago
|
||
Comment on attachment 8787919 [details] [diff] [review]
1069012-unprefix-placeholder-pseudoelement.diff
In devtools/shared/css-properties-db.js, should ":placeholder" really be sorted in with the moz-prefixed things? I'd guess not.
In layout/inspector/tests/test_getCSSPseudoElementNames.html it almost certainly should _not_ be at the end, but rather before all the moz-prefixed bits.
>+++ b/layout/style/nsCSSParser.cpp
>+ // We currently allow :-moz-placeholder and ::-moz-placeholder and
>+ // ::placeholder. We have to // be a bit stricter regarding the
Stray "//" in there.
>+++ b/layout/style/test/test_selectors.html
>+ should_serialize_to("input::placeholder", "input::placeholder");
Should have a test for ::-moz-placeholder here too, to make sure it doesn't serialize as ::placeholder, right?
r=me, but please send an intent to ship and file a followup to remove ::-moz-placeholder.
Attachment #8787919 -
Flags: review?(bzbarsky) → review+
Updated•8 years ago
|
Keywords: site-compat
Assignee | ||
Comment 11•8 years ago
|
||
Thanks, Boris. I just an intent-to-ship (https://groups.google.com/forum/?_escaped_fragment_=topic/mozilla.dev.platform/FkAJkVOJF74#!topic/mozilla.dev.platform/FkAJkVOJF74) and will address your review comments as soon as possible.
Assignee | ||
Comment 12•8 years ago
|
||
Alright, here's a fresh version of the patch that addresses your comments. Note that there was already an existing test for ::-moz-placeholder serialization (a few lines beneath that one), so I didn't have to add that.
Since the changes were trivial and non-functional, and the patch still applies cleanly to inbound, I'm carrying over r+ and requesting checkin.
Attachment #8787919 -
Attachment is obsolete: true
Assignee | ||
Updated•8 years ago
|
Keywords: checkin-needed
Comment 13•8 years ago
|
||
landed as http://hg.mozilla.org/integration/mozilla-inbound/rev/b31b1d10f122 but pulsebot failed to comment
Keywords: checkin-needed
Comment 14•8 years ago
|
||
bugherder |
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
status-firefox51:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla51
Comment 15•8 years ago
|
||
Posted the site compatibility doc: https://www.fxsitecompat.com/en-CA/docs/2016/placeholder-pseudo-element-and-placeholder-shown-pseudo-class-have-been-unprefixed/
Comment 16•8 years ago
|
||
Documented the changes:
https://developer.mozilla.org/en-US/docs/Web/CSS/:placeholder-shown
https://developer.mozilla.org/en-US/docs/Web/CSS/::placeholder
https://developer.mozilla.org/en-US/docs/Web/CSS/::-moz-placeholder
https://developer.mozilla.org/en-US/docs/Web/CSS/:-moz-placeholder
https://developer.mozilla.org/en-US/Firefox/Releases/51
Sebastian
Keywords: dev-doc-needed → dev-doc-complete
You need to log in
before you can comment on or make changes to this bug.
Description
•