286895 - [FIX]certain seemingly-irrelevant userContent.css settings break forms completely

Status: RESOLVED FIXED

(Core :: CSS Parsing and Computation, defect)

Description

[Silas S. Brown]

User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.0.0) Gecko/20020623 Debian/1.0.0-0.woody.1
Build Identifier: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.0.0) Gecko/20020623 Debian/1.0.0-0.woody.1

If you put the following in userContent.css:

div:first-letter { color: inherit ! important;
                   background: inherit ! important; }

then all TEXTAREAs will not work properly (try typing something, press
Enter, then continue typing - various strange things happen to the text
you are entering).  This does NOT happen when using :first-letter with
any other element, only DIV (and the universal selector which includes
DIV).  I tested on a blank page with only a TEXTAREA.

Additionally, drop-down boxes sometimes do not display properly
(depending on your other CSS settings e.g. colours and sizes of fonts) -
I was unable to make a complete catalogue of when this does/doesn't
happen but you can see some of the effects using ONLY the above in your
userContent.css.  Ensuring that your userContent.css does not override
DIV:first-letter (either directly or indirectly) seems to solve ALL the
problems.

Additionally, if you override :first-line (which includes TD:first-line,
TR:first-line etc) then the cursor can be positioned completely wrongly
when typing data into input boxes that occur within tables (the cursor
can be placed on another form field or in a seemingly random place
instead of in the box you're typing), and your text can be shown in the
wrong place also.


Reproducible: Always

Steps to Reproduce:
See above

Actual Results:  
See above

Expected Results:  
worked properly

You can see my complete stylesheets at
http://www.cus.cam.ac.uk/~ssb22/source/lv-css.html but again I emphasize
that the above test works with only the given snippet in userContent.css
and nothing else.

Comment 1

[Hixie (not reading bugmail)]

<textarea>s contain <div>s in Mozilla. And userContent.css applies to those
<div>s. (Normal author CSS doesn't.)

Comment 2

[Boris Zbarsky [:bzbarsky]]

Yeah... perhaps we shouldn't apply user stylesheets to native anonymous content?
 I guess the problem is people styling scrollbars?

Comment 3

[[e-mail]]

Assuming we ever manage to make scrollbars native anonymous...

Perhaps we need a userScrollbars.css ;-)

Comment 4

[Bob Smith]

Simply add "div {background-color: red;}" to userContent.css and all your
<Textarea> and <input type="text" ... > controls will have a red background.

But the expected result is a white background (the default).

I don't see what this has to do with scrollbars. I am not trying to style any
scrollbars, but I want my DIVs and Ps to have a certain background color and I
want my TEXTAREAs and INPUTs to have a different background color.

Adding a css rule for DIV in userContent.css should not affect a textarea or
text input control.

Comment 5

[Bob Smith]

I forgot to mention that this happens with
Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.7.9) Gecko/20050713
Firefox/1.0.5
and also
Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.8b2) Gecko/20050701

Comment 6

[Boris Zbarsky [:bzbarsky]]

See comment 1, please.

Comment 7

[Bob Smith]

And please see <a href="#c2">comment 2</a> for the solution, please.
I don't think that this is still an unconfirmed bug, if several testcases are
presented and if Ian Hickson gives the reason for this bug.
Thank you for your time.

Comment 8

[Boris Zbarsky [:bzbarsky]]

Comment 2 is not a solution.  Comment 2 is part of the problem description.  I
should know; I made that comment.

This is unconfirmed because we haven't decided whether it's invalid, and for
layout the idea is that you confirm valid bugs.

Comment 9

[Bob Smith]

So let me make this clear:

I want to change the look and feel of element A. I do so and suddenly also the
look and feel of element B changes.

Expected behavior: ONLY element A should change, since that is what I specified.

You think that is not bug?? Well - you may as well close this bug then.

Where I work we fix bugs and make our customers happy by doing so.

Comment 10

[Boris Zbarsky [:bzbarsky]]

Rather, you change element A and B changes because B depends on A.  The fact
that you didn't realize that doesn't make it a bug in the layout engine, just a
matter of education.

Back to the question in comment 2.  Do people ever style scrollbars for
<textarea> and such via userContent.css?

Comment 11

[Bob Smith]

From a web developer perspective a "<textarea>" has nothing to do with an
"<input type="text" ... >". These are two completely different html elements.

That internally you are using a textarea to create an input field is Mozilla's
problem. And not my problem.

When I specify that I want to modify the behavior of html element A (a textarea)
then a completely unrelated html element B (an input field) should not change.

Comment 12

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

We should fix this.

The internals of our form controls should be exposed as little as possible
unless we want them to be a stable API.

Comment 13

[Boris Zbarsky [:bzbarsky]]

Then we should also disable styling of form control and document scrollbars via
userContent.css and be done with it....

Comment 14

[Bob Smith]

Disabling or removing a feature is not how you fix bugs.

Do not apply user stylesheets to native anonymous content is the solution.

David - thank you for your support.

Comment 15

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

(In reply to comment #13)
> Then we should also disable styling of form control and document scrollbars via
> userContent.css and be done with it....

Agreed, at least until we have a potentially-standardizable solution for styling
them.

Comment 16

[Boris Zbarsky [:bzbarsky]]

> Do not apply user stylesheets to native anonymous content is the solution.

Scrollbars ARE native anonymous content.  So we would be "disabling or removing
a feature".

Another question.  What about user style sheets and XBL anonymous content (so
not native anonymous).  This has exactly the same issues, it seems to me. 
Unfortunately I suspect people won't see it that way...

Comment 17

[Boris Zbarsky [:bzbarsky]]

OK.  So given bug 164617 scrollbars are perhaps not an issue.  What about XBL?

That is, should userContent.css and userChrome.css apply to XBL anonymous content when the binding says that document sheets should not apply to it?

I'm tempted to say "no".  But that lets page authors create chunks of content not stylable via userContent.css, which seems undesirable...

Comment 18

[Boris Zbarsky [:bzbarsky]]

Attachment: Fix

The nsStyleSet changes fix the bug.  The rest is just to make it easy to catch bogus binding parents....

Oh, and of course for XUL document's and scrollbars this won't help, since the frame constructor special-cases that.  Have I mentioned lately how much I hate XUL?

Comment 19

[Boris Zbarsky [:bzbarsky]]

Oh, I left user*.css applying to XBL anonymous content.

Which means that they _might_ apply to the insides of scrollbars.  To fix that I'd have to actually walk the bindingParent() chain to see whether I ever get the same node.  I suppose I could do that, though.  Let me know?

Comment 20

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Comment on attachment 252029 [details] [diff] [review]
Fix

We shouldn't apply author styles in these cases either.  Could you at least assert that what the binding manager returns always includes this result (in other words, we never apply author styles when we don't apply user styles) -- although it might be preferable to check it explicitly.  r+sr=dbaron with that

Is the XXXldb comment in nsBindingManager::WalkRules something we could do?  Should we file a bug on that?

Comment 21

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

(In reply to comment #20)
> Is the XXXldb comment in nsBindingManager::WalkRules something we could do? 
> Should we file a bug on that?

The comment was only in my tree, and it said:
  Could we use nsIContent::IsNativeAnonymous instead of this hack?
("this hack" being the GetBindingParent returning the same node).

Comment 22

[Boris Zbarsky [:bzbarsky]]

Added a check for !skipUserStyles in addition to the existing !cutOffInheritance checks, and fixed.  Not sure how to test this...

Comment 23

[Boris Zbarsky [:bzbarsky]]

Ah, yeah.  We can't do that, because IsNativeAnonymous() lies for XUL.  See the XUL checks in the asserts in this patch.  :(

Comment 24

[Adam Guthrie]

Would adding some sort of functionality to Mochitest to create a userContent.css file (and add rules to it) be appropriate here, or is there another way to do this? Not sure how we'd handle this since it'd require a browser restart to pick up these rules...