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)

defect
Not set
normal

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)

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.
Why change the name?
The CSS working group resolved on these names after a long discussion; I wouldn't reopen that discussion without a very good reason.
Chrome and WebKit supported.
@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
::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]
(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
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: nobody → wisniewskit
Status: NEW → ASSIGNED
Attachment #8787829 - Flags: review?(bzbarsky)
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 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+
Keywords: site-compat
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.
Blocks: 1300896
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
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/b31b1d10f122
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla51
Blocks: 1301498
Blocks: 1301499
You need to log in before you can comment on or make changes to this bug.