Closed Bug 888884 Opened 11 years ago Closed 4 years ago

:-moz-read-write pseudo-class shouldn't be applied on <input type=text disabled> and <textarea disabled>

Categories

(Core :: Layout: Form Controls, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla78
Tracking Status
firefox78 --- fixed

People

(Reporter: m_kato, Assigned: emilio)

References

()

Details

(Keywords: dev-doc-complete)

Attachments

(3 files, 1 obsolete file)

WHATWG spec says,

The :read-write pseudo-class must match any element falling into one of the following categories, which for the purposes of Selectors are thus considered user-alterable: [SELECTORS]

    input elements to which the readonly attribute applies, and that are mutable (i.e. that do not have the readonly attribute specified and that are not disabled)


But :-moz-read-write is applyed on <input type=text disabled>.

Same issue occurs on Blink and WebKit. (http://code.google.com/p/chromium/issues/detail?id=255351)
The spec here may well be bogus; I recall there being issues raised on it in the past.

In any case, this is a pure form controls bug: the element claims to be in that state, so the selector matches it.  There's nothing related to CSS proper going on here.  See the IntrinsicState methods.
Component: CSS Parsing and Computation → Layout: Form Controls
Actually, we are thinking of simply removing the :read-write and :read-only selectors from HTML. Last I've heard, Hixie is was asking the CSS editors if they would agree to remove the selectors from CSS specifications.

If that happen, the specifications would not matter much.

See: https://www.w3.org/Bugs/Public/show_bug.cgi?id=17812
Comment on attachment 8693464 [details] [diff] [review]
Part 1. - Apply :-moz-read-only on disabled edtiable text

Don't set NS_EVENT_STATE_MOZ_READWRITE that editable element has disabled attirubte.
Attachment #8693464 - Flags: review?(bzbarsky)
Comment on attachment 8693465 [details] [diff] [review]
Part 2. Add reftest

Add reftest for input type="text" and textarea
Attachment #8693465 - Attachment description: Bug 888884 - Part 2. Add reftest → Part 2. Add reftest
Attachment #8693465 - Flags: review?(bzbarsky)
Comment on attachment 8693464 [details] [diff] [review]
Part 1. - Apply :-moz-read-only on disabled edtiable text

What was the outcome of the spec discussion?

What sort of checking have you done to make sure this won't break content, both on the web and addons?
Flags: needinfo?(m_kato)
Comment on attachment 8693465 [details] [diff] [review]
Part 2. Add reftest

r=me assuming we make this change.
Attachment #8693465 - Flags: review?(bzbarsky) → review+
(In reply to Boris Zbarsky [:bz] from comment #7)
> Comment on attachment 8693464 [details] [diff] [review]
> Part 1. - Apply :-moz-read-only on disabled edtiable text
> 
> What was the outcome of the spec discussion?
> 
> What sort of checking have you done to make sure this won't break content,
> both on the web and addons?

- Blink wants to keep same behavior as Firefox and old Opera.  It is for web compatibility. (https://code.google.com/p/chromium/issues/detail?id=255351)
- WebKit already changed to HTML5 spec's behavior (disabled applys :read-only) at 2 year ago.  But I cannot find break the web by this change. 
- HTML5 spec unfortunately says as same as WHATWG spec (http://www.w3.org/TR/html5/disabled-elements.html).

Also ,we still use prefix for this. But Blink and WebKit doesn't use prefix and both is different behavior now.
Flags: needinfo?(m_kato)
Comment on attachment 8693464 [details] [diff] [review]
Part 1. - Apply :-moz-read-only on disabled edtiable text

> - Blink wants to keep same behavior as Firefox and old Opera.

Or at least one Blink developer does.  What about IE?

I would really like us to change this no more than once.  Please talk to the Blink developers about whether they're willing to get on board and make this change if we do, so we don't end up having to revert this change later.

Please re-request review once we have a commitment from Blink, ok?
Attachment #8693464 - Flags: review?(bzbarsky)
(In reply to Boris Zbarsky [:bz] from comment #10)
> Comment on attachment 8693464 [details] [diff] [review]
> Part 1. - Apply :-moz-read-only on disabled edtiable text
> 
> > - Blink wants to keep same behavior as Firefox and old Opera.
> 
> Or at least one Blink developer does.  What about IE?

Edge 13 is first implementation for it and is same as Blink.

> I would really like us to change this no more than once.  Please talk to the
> Blink developers about whether they're willing to get on board and make this
> change if we do, so we don't end up having to revert this change later.

OK.  I should discuss it with google.  Maybe, we should change the spec (HTML5.1, CSS selector 4 and WHATWG) after discussion?

> Please re-request review once we have a commitment from Blink, ok?

OK.
Blocks: 312971
> Maybe, we should change the spec (HTML5.1, CSS selector 4 and WHATWG) after discussion?

If no one except Safari is willing to implement it, then yes...
Summary: :-moz-read-write pseudo-class shouldn't be applied on <input type=text disabled> → :-moz-read-write pseudo-class shouldn't be applied on <input type=text disabled> and <textarea disabled>

Per https://html.spec.whatwg.org/#selector-read-write:

The :read-write pseudo-class must match any element falling into one
of the following categories [...]:

  • input elements to which the readonly attribute applies, and that
    are mutable (i.e. that do not have the readonly attribute
    specified and that are not disabled)

  • textarea elements that do not have a readonly attribute, and
    that are not disabled.

  • elements that are editing hosts or editable and are neither
    input elements nor textarea elements.

This fixes the :disabled bits.

This matches Safari behavior and the spec, but not Chrome, which has our
behavior.

Fix the WPT, which had multiple issues, as :read-only is defined to be
just the opposite of :read-write. This will pass as soon as I unprefix
the pseudo-class.

Assignee: m_kato → emilio
Pushed by ealvarez@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/b891abd233e6
Make :read-write not apply for disabled controls to which readonly applies. r=edgar
Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/23603 for changes under testing/web-platform/tests
Upstream web-platform-tests status checks passed, PR will merge once commit reaches central.
Pushed by emilio@crisal.io:
https://hg.mozilla.org/integration/autoland/rev/e1eb80ef4079
Mark the newly added WPTs as failing for now.

As the pseudo-class is still not un-prefixed.

Attachment #9149066 - Attachment is obsolete: true
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla78
Upstream PR merged by moz-wptsync-bot

Mentioned this in the site compatibility note for Bug 312971.

Keywords: dev-doc-needed

I've added a rel note to cover this: https://wiki.developer.mozilla.org/en-US/docs/Mozilla/Firefox/Releases/78#CSS

I don't think anything else is needed here.

You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: