Last Comment Bug 302188 - Support :-moz-read-only and :-moz-read-write pseudoclasses
: Support :-moz-read-only and :-moz-read-write pseudoclasses
Status: RESOLVED FIXED
: css3, fixed1.8, qawanted
Product: Core
Classification: Components
Component: CSS Parsing and Computation (show other bugs)
: Trunk
: All All
: -- enhancement (vote)
: ---
Assigned To: Allan Beaufour
: Hixie (not reading bugmail)
Mentors:
http://www.w3.org/TR/css3-ui/#pseudo-...
Depends on:
Blocks: 271720
  Show dependency treegraph
 
Reported: 2005-07-26 05:28 PDT by Allan Beaufour
Modified: 2011-08-05 22:36 PDT (History)
18 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Patch (13.24 KB, patch)
2005-07-26 07:49 PDT, Allan Beaufour
no flags Details | Diff | Splinter Review
Testcase (4.67 KB, application/xhtml+xml)
2005-07-28 02:10 PDT, Allan Beaufour
no flags Details
Patch v2 (29.44 KB, patch)
2005-08-03 05:54 PDT, Allan Beaufour
no flags Details | Diff | Splinter Review
Patch that implements :-moz-read-only and :-moz-read-write (5.69 KB, patch)
2005-10-14 02:11 PDT, Allan Beaufour
bzbarsky: review+
bzbarsky: superreview+
asa: approval1.8rc1+
Details | Diff | Splinter Review
diff -w (3.95 KB, patch)
2005-10-17 01:31 PDT, Allan Beaufour
no flags Details | Diff | Splinter Review

Description Allan Beaufour 2005-07-26 05:28:26 PDT
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
Comment 1 Allan Beaufour 2005-07-26 07:49:47 PDT
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.
Comment 2 Allan Beaufour 2005-07-26 07:54:04 PDT
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.)"
Comment 3 Boris Zbarsky [:bz] 2005-07-26 08:27:36 PDT
> 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.
Comment 4 Allan Beaufour 2005-07-27 06:40:09 PDT
(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.
Comment 5 Allan Beaufour 2005-07-27 07:21:51 PDT
(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;

?
Comment 6 Allan Beaufour 2005-07-27 07:25:01 PDT
... needs to check readonly state of designMode too
Comment 7 Boris Zbarsky [:bz] 2005-07-27 08:35:05 PDT
> 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.
Comment 8 Allan Beaufour 2005-07-28 02:10:45 PDT
Created attachment 190820 [details]
Testcase
Comment 9 Allan Beaufour 2005-07-28 02:19:39 PDT
(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
Comment 10 Allan Beaufour 2005-07-28 02:51:58 PDT
(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.
Comment 11 Boris Zbarsky [:bz] 2005-07-28 09:03:05 PDT
Or checking for the attr first and QIing if it's set...  That would be faster.
Comment 12 Allan Beaufour 2005-07-28 23:21:17 PDT
(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.
Comment 13 Allan Beaufour 2005-07-29 04:53:09 PDT
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.
Comment 14 Allan Beaufour 2005-07-29 05:21:33 PDT
(In reply to comment #1)
> Created an attachment (id=190557) [edit]
> Patch

Hmmm, I need to disable matching for button and select.
Comment 15 Boris Zbarsky [:bz] 2005-07-29 07:37:31 PDT
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....
Comment 16 Boris Zbarsky [:bz] 2005-07-29 10:52:47 PDT
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...
Comment 17 Daniel Glazman (:glazou) 2005-08-01 04:06:02 PDT
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.
Comment 18 Allan Beaufour 2005-08-03 05:54:23 PDT
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.
Comment 19 Allan Beaufour 2005-08-04 02:12:59 PDT
(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 :)
Comment 20 Daniel Glazman (:glazou) 2005-08-04 02:22:54 PDT
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.
Comment 21 Boris Zbarsky [:bz] 2005-08-04 10:13:01 PDT
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.)
Comment 22 Mike Connor [:mconnor] 2005-08-04 12:47:11 PDT
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 neil@parkwaycc.co.uk 2005-08-04 12:55:54 PDT
(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?
Comment 24 Boris Zbarsky [:bz] 2005-08-04 13:22:22 PDT
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.
Comment 25 Allan Beaufour 2005-09-01 14:30:46 PDT
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)?
Comment 26 Boris Zbarsky [:bz] 2005-09-01 19:00:00 PDT
I'd really rather not parse these if we don't do anything with them....
Comment 27 Daniel Glazman (:glazou) 2005-09-02 00:23:52 PDT
(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.
Comment 28 Allan Beaufour 2005-09-02 07:51:16 PDT
(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 :)
Comment 29 Boris Zbarsky [:bz] 2005-09-02 08:22:06 PDT
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...
Comment 30 Daniel Glazman (:glazou) 2005-09-02 08:26:52 PDT
(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.
Comment 31 Allan Beaufour 2005-09-02 09:49:18 PDT
(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.
Comment 32 Boris Zbarsky [:bz] 2005-09-02 11:29:10 PDT
> that it?

Yes.

> that means that we cannot gradually implement CSS 3 pseudoclasses

You can implement them one pseudo-class at a time...
Comment 33 Allan Beaufour 2005-09-02 12:09:42 PDT
(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."
Comment 34 Boris Zbarsky [:bz] 2005-09-02 12:16:08 PDT
> 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.
Comment 35 Allan Beaufour 2005-09-02 12:43:35 PDT
(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...
Comment 36 Hixie (not reading bugmail) 2005-09-04 03:51:34 PDT
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.
Comment 37 Allan Beaufour 2005-09-14 01:26:57 PDT
(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?
Comment 38 Allan Beaufour 2005-09-27 07:22:41 PDT
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)...
Comment 39 aaronr 2005-10-12 17:02:57 PDT
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 Darin Fisher 2005-10-12 17:07:05 PDT
I'm pretty sure this would not be approved for 1.5 since we're nearing the first
candidate "1.5 final" release.
Comment 41 Boris Zbarsky [:bz] 2005-10-13 20:37:28 PDT
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.
Comment 42 Allan Beaufour 2005-10-14 01:37:20 PDT
(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?
Comment 43 Allan Beaufour 2005-10-14 02:11:44 PDT
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?
Comment 44 Boris Zbarsky [:bz] 2005-10-14 05:59:25 PDT
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?
Comment 45 Allan Beaufour 2005-10-14 06:03:31 PDT
(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.
Comment 46 Allan Beaufour 2005-10-14 06:09:33 PDT
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.
Comment 47 Brendan Eich [:brendan] 2005-10-14 09:40:13 PDT
How about a diff -w just to show the minimal changes?

/be
Comment 48 Asa Dotzler [:asa] 2005-10-14 12:29:07 PDT
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.
Comment 49 Boris Zbarsky [:bz] 2005-10-16 11:15:29 PDT
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.
Comment 50 Allan Beaufour 2005-10-17 01:31:02 PDT
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 Asa Dotzler [:asa] 2005-10-17 11:14:46 PDT
Comment on attachment 199534 [details] [diff] [review]
Patch that implements :-moz-read-only and :-moz-read-write

we've been overruled by brendan. approved.
Comment 52 Doron Rosenberg (IBM) 2005-10-17 11:40:35 PDT
checked into branch
Comment 53 Brendan Eich [:brendan] 2005-10-17 13:29:54 PDT
(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
Comment 54 Doron Rosenberg (IBM) 2005-10-18 14:17:02 PDT
checked into trunk
Comment 55 Allan Beaufour 2005-10-19 00:39:15 PDT
I've created bug 312971 for :read-only/:read-write
Comment 56 Boris Zbarsky [:bz] 2006-02-10 13:17:16 PST
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.
Comment 57 Allan Beaufour 2006-03-02 00:04:59 PST
(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?
Comment 58 Boris Zbarsky [:bz] 2006-03-02 00:15:12 PST
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.
Comment 59 Allan Beaufour 2006-03-08 04:36:20 PST
(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

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