Support :-moz-read-only and :-moz-read-write pseudoclasses

RESOLVED FIXED

Status

()

Core
CSS Parsing and Computation
--
enhancement
RESOLVED FIXED
12 years ago
6 years ago

People

(Reporter: Allan Beaufour, Assigned: Allan Beaufour)

Tracking

({css3, fixed1.8, qawanted})

Trunk
css3, fixed1.8, qawanted
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(URL)

Attachments

(4 attachments, 1 obsolete attachment)

(Assignee)

Description

12 years ago
We should support the :read-only and :read-write pseudoclasses defined in CSS3 UI:
http://www.w3.org/TR/css3-ui/#pseudo-ro-rw
and WebForms 2:
http://whatwg.org/specs/web-forms/current-work/#relation
(Assignee)

Updated

12 years ago
Status: NEW → ASSIGNED
(Assignee)

Comment 1

12 years ago
Created attachment 190557 [details] [diff] [review]
Patch

Does not handle:
* XUL
* editable documents

Both areas where I am quite green, so personally I would rather delay support
for that.

Anne has testcases coming up.
(Assignee)

Comment 2

12 years ago
Question is: Should read-only match "world"? WF2 now says:
":read-only
    Matches form control elements that have the readonly attribute set, and to
which the readonly attribute applies. (For instance, radio buttons will never
match this, regardless of the value of the attribute.)"
> Question is: Should read-only match "world"?

Per my reading of CSS3 UI, yes.  That is, they should match everything whose
state cannot be changed by the user...

> WF2 now says:

That's only talking about form controls in general, of course.  A random <div>
should certainly match readonly, imo.

And I'd like this working at least for editable documents before it's landed. 
It shouldn't be too hard to make it work for XUL either, fwiw... Certainly if
you're going to be parsing/matching this selector at all in editable documents
and XUL you want to be doing it right.
(Assignee)

Comment 4

12 years ago
(In reply to comment #3)
> And I'd like this working at least for editable documents before it's landed.

Hints appreciated... it's yet another new area for me.

> It shouldn't be too hard to make it work for XUL either, fwiw... Certainly if
> you're going to be parsing/matching this selector at all in editable documents
> and XUL you want to be doing it right.

Then we're back to figuring out where to expose scriptable functionality I
think. Here's what's on top of my mind:

All XUL elements should be able to set whatever pseudoclasses they want, and
they need to be able to probe existing elements. xul:textbox needs to be able to
probe html:input and return its intrinsic state. Something like nsIDOMNSElement
{ readonly attribute long intrinsicState; } and nsIDOMXULElement { attribute
long intrinsicState; }. Setting it would trigger notifications.
(Assignee)

Comment 5

12 years ago
(In reply to comment #4)
> (In reply to comment #3)
> > And I'd like this working at least for editable documents before it's landed.
> 
> Hints appreciated... it's yet another new area for me.

Something like this:
nsCOMPtr<nsIDOMNSHTMLDocument> doc =
  do_QueryInterface(GetCurrentDoc());

if (doc) {
  nsAutoString designMode;
  doc->GetDesignMode(designMode);

  if (designMode.EqualsLiteral("on")) {
    return NS_EVENT_STATE_READWRITE;
  }
}

return NS_EVENT_STATE_READONLY;

?
(Assignee)

Comment 6

12 years ago
... needs to check readonly state of designMode too
> Hints appreciated... it's yet another new area for me.

Most likely, a function on nsIDocument to check.  Have to be careful; I'm pretty
worried by the perf impact of all this (another reason I'm happier with separate
patches).

> Then we're back to figuring out where to expose scriptable functionality

Why?  Don't the various XUL "form controls" implement appropriate interfaces so
you can check?  I thought they did...  Though I guess that comes back to the
perf concern.  :(

> Something like this:

That works for designMode, but not for a document being edited in an editor.
(Assignee)

Comment 8

12 years ago
Created attachment 190820 [details]
Testcase
(Assignee)

Comment 9

12 years ago
(In reply to comment #7)
> > Then we're back to figuring out where to expose scriptable functionality
> 
> Why?  Don't the various XUL "form controls" implement appropriate interfaces so
> you can check?  I thought they did...  Though I guess that comes back to the
> perf concern.  :(

Well, they sort of do, but it will not be the most flexible solution. But I
guess, we could wait and save the world in 1.9 :) I'll just check the readonly
attribute then.

> > Something like this:
> 
> That works for designMode, but not for a document being edited in an editor.

hmmm...

/me wonders how that works and dives into lxr
(Assignee)

Comment 10

12 years ago
(In reply to comment #9)
> (In reply to comment #7)
> > > Then we're back to figuring out where to expose scriptable functionality
> > 
> > Why?  Don't the various XUL "form controls" implement appropriate interfaces so
> > you can check?  I thought they did...  Though I guess that comes back to the
> > perf concern.  :(
> 
> Well, they sort of do, but it will not be the most flexible solution. But I
> guess, we could wait and save the world in 1.9 :) I'll just check the readonly
> attribute then.

Hmmm, I guess that would mean QI'ing to nsIDOMXULTextBoxElement, and if it
succeeds checking for the readonly attribute? I'm not sure I like that.
Or checking for the attr first and QIing if it's set...  That would be faster.
(Assignee)

Comment 12

12 years ago
(In reply to comment #11)
> Or checking for the attr first and QIing if it's set...  That would be faster.

Problem is that it's the reverse situation: I have to know whether the attribute
is not there, specifically on a xul:textbox.
(Assignee)

Comment 13

12 years ago
I've looked a bit at the XUL handling, and not really at the "document being
inside an editor".... I would rather do a follow-up bug for those two issues.
(Assignee)

Comment 14

12 years ago
(In reply to comment #1)
> Created an attachment (id=190557) [edit]
> Patch

Hmmm, I need to disable matching for button and select.
If we were doing this sometime in an alpha, we would have time for a followup
bug.  As things stand, we're planning to branch any minute now, hence my
insistence on getting things right as they land....
Ah, true...  Not quite sure what an optimal solution here would be.  Do we have
any free bits or something?  If we had a bit we could use for whether |readonly|
applies, that would be great...
I have not investigated a lot but I think that those pseudos should be disabled
in the editor. Having them enabled, while they really apply to browsing mode,
could turn editing into hell: imagine that you are preparing a document where a
few elements become contenteditable and you add the rule

  *:read-write { border: 1px dotted red }

I let you imagine the effect if it applies into the editor... Borders everywhere.

Then I suggest to restrict the application of those pseudos to browsing mode.
(Assignee)

Comment 18

12 years ago
Created attachment 191469 [details] [diff] [review]
Patch v2

This one passes all Anne's testcases and includes support for XUL textbox and
textarea. Instead of QI'ing I check the namespace and tagname of the xul
element. Not the most flexible, but fast.

I'm sure whether I need to check the namespace though, as nsXULElement should
always live in the XUL NS I guess?

For editor ... a reaction on comment 17? I would tend to agree with Daniel (and
not _just_ because I'm lazy :) ). When you "WYSIWYG-edit" a document, I guess
it should look like as it will when being viewed.
Attachment #190557 - Attachment is obsolete: true
Attachment #191469 - Flags: review?(bzbarsky)
(Assignee)

Comment 19

12 years ago
(In reply to comment #18)
> For editor ... a reaction on comment 17? I would tend to agree with Daniel (and
> not _just_ because I'm lazy :) ). When you "WYSIWYG-edit" a document, I guess
> it should look like as it will when being viewed.

*ahem* That was actually not what Daniel said. Oh well, so I disagree with
everybody and say that there should be no difference :)
That's the contrary, **in particular if you are editing a page that will be
partly editable in the browser**. You may want to let the BROWSER USER that some
specific areas are editable, while ALL areas are editable for the EDITOR USER...
If :read-write is used to outline those areas, everything will be outlined in
the editor, resulting in a mess.
Just checking the tagname is no good for XUL, since what matters is what binding
is applied, not what tagname is used.

David, Neil, Mike, do you think we should try to get this working for XUL before
landing?  Or just make it work for HTML and XForms and do XUL later?  In the
former case, I suspect this immediately becomes 1.9 material; in the latter
case, we might be able to get this in for 1.8, maybe... (David, I'd most like to
hear your opinion here.)
As long as there's a committment to fixing XUL for 1.9, I'm not overly concerned
about when HTML gets support.  If its possible and safe for 1.8, it'd be nice to
give web authors another toy.

Comment 23

12 years ago
(In reply to comment #21)
>David, Neil, Mike, do you think we should try to get this working for XUL
What (if anything) do we gain by getting this working for XUL?
We gain the ability to use these selectors; right now we have to use attribute
selectors instead.  This allows user stylesheets to not have to do special cases
for remote XUL, at the very least.
(Assignee)

Comment 25

12 years ago
I see two ways out of this:
1) Either implement :read-only/:read-write as no-op until the issues are sorted out

2) Implement them as suggested in:
http://lists.w3.org/Archives/Public/www-style/2005Aug/0156.html

Although I actually am positive to 2), we might be doing what we wanted to
avoid: Implement something that will change later. We could really use these two
pseudoclasses in XForms -- it's the last two we need, so 1)?
I'd really rather not parse these if we don't do anything with them....
(In reply to comment #26)

> I'd really rather not parse these if we don't do anything with them....

This is clearly something we did in the past and we must stop. Each time we say
"we're not going to add that to the parser because it's not (well) implemented",
it means
that we're unable to edit stylesheets using that selector/property/value/whatever.
It's a *major* problem for Nvu and I highly prefer having :read-write and
:read-only (a) parsed (b) always returning false in SelectorMatches()
(c) serialized.

That's exactly the same thing when we name a property -moz-foo instead of foo.
That's very cool to show we're not yet conformant to the spec, but it's a
blocker for Nvu that is then not able to parse and preserve foo.
(Assignee)

Comment 28

12 years ago
(In reply to comment #27)
> (In reply to comment #26)
> > I'd really rather not parse these if we don't do anything with them....

That depends on the definition of "we". As in browser core, then yes, "we" don't
do anything with them. But XForms will use them, and apparently also NVU.

> This is clearly something we did in the past and we must stop. Each time we
> say "we're not going to add that to the parser because it's not (well)
> implemented", it means that we're unable to edit stylesheets using that
> selector/property/value/whatever.

Another user... weee :)
Daniel, the problem is that given a selector like:

div, input:read-only {
  rules
}

If we don't support :read-only we should drop the whole rule per the CSS spec. 
So it's not just a matter of the :read-only not matching in SelectorMatches; the
"div" selector ALSO needs to not match.

We might be able to do this with a flag on selectors of some sort that indicates
that they came from a selector list that we actually failed to parse.  But that
would only help in a very few special cases like this; for example:

  div, input:nth-of-type(3)

would still just get dropped, since we can't make sense of the CSS3 selectors
that have args yet...
(In reply to comment #29)
> Daniel, the problem is that given a selector like:
> 
> div, input:read-only {
>   rules
> }
> 
> If we don't support :read-only we should drop the whole rule per the CSS spec.

Boris, you perfectly know I have always fought that because if it's perfect
for a browser, it's plain stupid for an editor... I remind you the CSS OM has
CSSUnknownRule for that reason. Yeah, I know, the parsing rules of CSS don't
even allow the create of such an object.
(Assignee)

Comment 31

12 years ago
(In reply to comment #29)
> If we don't support :read-only we should drop the whole rule per the CSS spec. 

Which section are you refering to in the spec.
"When a user agent can't parse the selector (i.e., it is not valid CSS 2.1), it
must ignore the {}-block as well."
(http://www.w3.org/TR/2005/WD-CSS21-20050613/syndata.html#q10)
that it?

If I read it correctly, that means that we cannot gradually implement CSS 3
pseudoclasses. It's all or nothing.

Daniel has written to the WG mailing list about it.
> that it?

Yes.

> that means that we cannot gradually implement CSS 3 pseudoclasses

You can implement them one pseudo-class at a time...
(Assignee)

Comment 33

12 years ago
(In reply to comment #32)
> > that it?
> 
> Yes.
> 
> > that means that we cannot gradually implement CSS 3 pseudoclasses
> 
> You can implement them one pseudo-class at a time...

Wouldn't that break this:
"When a user agent can't parse the selector (i.e., it is not valid CSS 2.1), it
must ignore the {}-block as well."
> When a user agent can't parse the selector (i.e., it is not valid CSS 2.1)

For a UA that claims no compliance to anything but CSS2.1, that would be
correct.  But we're implementing part of CSS3 Selectors, not CSS2.1, here.
(Assignee)

Comment 35

12 years ago
(In reply to comment #34)
> > When a user agent can't parse the selector (i.e., it is not valid CSS 2.1)
> 
> For a UA that claims no compliance to anything but CSS2.1, that would be
> correct.  But we're implementing part of CSS3 Selectors, not CSS2.1, here.

(nitpicking...) Ok, forget "gradually". What I was trying to say, is that if a
UA follows the CSS2.1 spec, it should, per the above sentence, ignore blocks
with selectors that are not valid CSS 2.1. So blocks for CSS3UI selectors should
be ignored if the UA wants CSS2.1 compliance, as CSS3UI doesn't replace CSS2.1?
Or else it should be seen as compliance = "CSS2.1 patched with CSS3UI" :)

Or maybe I'm just getting lost in the CSS3 module circus...
I don't understand why people get so confused about CSS conformance.

It's common sense. There is no problem supporting more features than CSS Level 2
has, it just means you are supporting more features than CSS Level 2 has. It
means you aren't a CSS2.1-conforming-UA, it means you are _better_ than a
CSS2.1- conforming-UA.
(Assignee)

Comment 37

12 years ago
(In reply to comment #36)
> I don't understand why people get so confused about CSS conformance.
> 
> It's common sense. There is no problem supporting more features than CSS Level 2
> has, it just means you are supporting more features than CSS Level 2 has. It
> means you aren't a CSS2.1-conforming-UA, it means you are _better_ than a
> CSS2.1- conforming-UA.

I am nitpicking, etc. etc. Yes, it is common sense to not read that part of the
css 2.1 spec. literally when you implement css 3 ui, but that's not exactly the
idea of a spec. is it?
(Assignee)

Comment 38

12 years ago
This is the last pair of pseudoclasses that we need for XForms, and they seem to
have gotten stuck between that:
1) nobody can give a proper definition of how it should work for HTML in
editable content 
2) we should not parse the pseudoclasses before we handle them for HTML

No surprise :), I'm in favour of letting a bit go of 2)...

Updated

12 years ago
Blocks: 65133

Updated

12 years ago
No longer blocks: 65133

Comment 39

12 years ago
This bug appears to have stalled out without any direction on where we can
proceed from here.  bz, since it hasn't made trunk yet, is it dead as far as FF
1.5 is concerned or can we still save it?

I guess whether it is out of reach for FF 1.5 now or not, we still need to get
this in the trunk.  bz, can you reiterate what issues need to be addressed? 
After reading through this bug, they are no longer apparent.  Thanks.

Comment 40

12 years ago
I'm pretty sure this would not be approved for 1.5 since we're nearing the first
candidate "1.5 final" release.
There's no way this will make 1.5...  :(

What needs to be addressed is getting some agreement on exactly how this stuff
should be applying to HTML.  If we can't get that agreement, we should be
implementing this as -moz-read-only and -moz-read-write.
(Assignee)

Comment 42

12 years ago
(In reply to comment #41)
> There's no way this will make 1.5...  :(
> 
> What needs to be addressed is getting some agreement on exactly how this stuff
> should be applying to HTML.  If we can't get that agreement, we should be
> implementing this as -moz-read-only and -moz-read-write.

Ok, how about no-ops for that, could we get that in then?
(Assignee)

Comment 43

12 years ago
Created attachment 199534 [details] [diff] [review]
Patch that implements :-moz-read-only and :-moz-read-write

Ok, here's a patch that implements :-moz-read-only and :-moz-read-write as
noops. This is exactly the same as went in on bug 302462 and bug 302608 a long
time ago, on both trunk and branch.

This is not optimal, but would still be a big help for XForms. So pretty please
with sugar on top :), could we get these ones in on FF1.5?
Attachment #199534 - Flags: review?(bzbarsky)
Comment on attachment 199534 [details] [diff] [review]
Patch that implements :-moz-read-only and :-moz-read-write

Yeah, ok.  If even this would help XForms, let's do it.  We just need to make
it clear that pages SHOULD NOT use these styles, since we're going to remove
them in the next release, hopefully.  So I hope that it's useful for our
internal styling, right?
Attachment #199534 - Flags: superreview+
Attachment #199534 - Flags: review?(bzbarsky)
Attachment #199534 - Flags: review+
(Assignee)

Comment 45

12 years ago
(In reply to comment #44)
> (From update of attachment 199534 [details] [diff] [review] [edit])
> Yeah, ok.  If even this would help XForms, let's do it.  We just need to make
> it clear that pages SHOULD NOT use these styles, since we're going to remove
> them in the next release, hopefully.  So I hope that it's useful for our
> internal styling, right?

Both code-wise and internal styling. With this we can remove all the "bogus"
attribute setting and removal! Weee.
(Assignee)

Comment 46

12 years ago
Comment on attachment 199534 [details] [diff] [review]
Patch that implements :-moz-read-only and :-moz-read-write

Requesting approval1.8rc1.

It just adds CSS parsing of :-moz-read-only and :-moz-read-write, without
matching any elements. This is the same as for bug 302462 and bug 302608. So it
should be a "no-risk" patch.

It will be very valuable for XForms since we can then switch our styling to
pseudoclasses, instead of the "hacky" attributes we use right now.

Proper :read-only and :read-write support should still be done, but that has to
wait as there are too many unspecified issues.
Attachment #199534 - Flags: approval1.8rc1?
How about a diff -w just to show the minimal changes?

/be

Comment 48

12 years ago
Comment on attachment 199534 [details] [diff] [review]
Patch that implements :-moz-read-only and :-moz-read-write

It's too late in the game for adding new css features, especially ones not
critical to the functioning of our own application. At this stage, we can't
even afford the noise in the system of non-critical patches.
Attachment #199534 - Flags: approval1.8rc1? → approval1.8rc1-
Comment on attachment 199534 [details] [diff] [review]
Patch that implements :-moz-read-only and :-moz-read-write

Renominating, per mail brendan and I sent to drivers.
Attachment #199534 - Flags: approval1.8rc1- → approval1.8rc1?
(Assignee)

Comment 50

12 years ago
Created attachment 199784 [details] [diff] [review]
diff -w

(In reply to comment #47)
> How about a diff -w just to show the minimal changes?

There you go, diff -w of attachment 199534 [details] [diff] [review]

Comment 51

12 years ago
Comment on attachment 199534 [details] [diff] [review]
Patch that implements :-moz-read-only and :-moz-read-write

we've been overruled by brendan. approved.
Attachment #199534 - Flags: approval1.8rc1? → approval1.8rc1+
checked into branch
Keywords: fixed1.8
(In reply to comment #51)
> (From update of attachment 199534 [details] [diff] [review] [edit])
> we've been overruled by brendan. approved.

My mail argued that this patch is technically safe, and we've wasted more time
pushing back on it based on general principles, and it was supposed to go in
earlier but was held up by circumstances not the fault of the patch or its
authors.  So, to actually *save time* and *make things better*, I said "let's
let it in, with a warning that no more changes get such a pass."

That warning is now given.

I hope no one feels burned by my "override".  I'll use it sparingly in the
future, but I think it's necessary to think technically about patches, and it is
always necessary to think economically.  At some point the release train has to
close its doors, however -- this patch just barely made it.

/be
checked into trunk
Status: ASSIGNED → RESOLVED
Last Resolved: 12 years ago
Resolution: --- → FIXED
Summary: Support :read-only and :read-write pseudoclasses → Support :-moz-read-only and :-moz-read-write pseudoclasses
(Assignee)

Comment 55

12 years ago
I've created bug 312971 for :read-only/:read-write
(Assignee)

Updated

12 years ago
Attachment #191469 - Flags: review?(bzbarsky)
Did the testcases ever get added to the regression tests?  If not, they should be.  Please do that.  That includes the testcases Anne has posted links to in the bug, since he's said he's ok with adding them to the regression tests.
(Assignee)

Comment 57

12 years ago
(In reply to comment #56)
> Did the testcases ever get added to the regression tests?  If not, they should
> be.  Please do that.  That includes the testcases Anne has posted links to in
> the bug, since he's said he's ok with adding them to the regression tests.

Nope, they were not added. Were should they be checked in to?
Mm.... probably layout/html/tests/style/bugs in the source tree.  Add the files (bugNNN-1.html, bugNNN-2.html, etc) to the dir, and add the filenames to the rtest.lst in that dir.  Then check it all in.
(Assignee)

Comment 59

12 years ago
(In reply to comment #58)
> Mm.... probably layout/html/tests/style/bugs in the source tree.  Add the files
> (bugNNN-1.html, bugNNN-2.html, etc) to the dir, and add the filenames to the
> rtest.lst in that dir.  Then check it all in.

Nothing to check in here really. We only support :-moz-read-only and :-moz-read-write, the testcases cover :read-only and :read-write
You need to log in before you can comment on or make changes to this bug.