160403 - inherit and -moz-initial repeated when serializing shorthands (breaks roundtripping)

Status: RESOLVED FIXED

(Core :: DOM: CSS Object Model, defect)

Description

[Vladimir Ermakov]

When a property is set to inherit value, the value reported in the dom is
quadrippled: "inherit inherit inherit inherit"

Attaching the following testcase.
<div id="test" style="border-color: inherit;"></div>
<script>
   alert(document.getElementById('test').style.borderColor);
</script>

Comment 1

[Vladimir Ermakov]

Attachment: testcase

Comment 2

[Boris Zbarsky [:bzbarsky]]

This is a canonicalization issue, not a correctness one....  Basically, it would
be nice to have a version of TryFourSidesShorthand that took eCSSPropertyFoo
instead of indices into mOrder....

Comment 3

[Daniel Glazman (:glazou) (not active in Mozilla any more)]

damn !!! c14n is my hell...

Comment 4

[Vladimir Ermakov]

This behaviour is sumilar to what happens with shorthand properties, but
border-color is not a shorthand...

Comment 5

[Boris Zbarsky [:bzbarsky]]

border-color _is_ a shorthand.  It sets border-top-color, border-right-color, etc.

Comment 6

[Vladimir Ermakov]

then perhaps this is not a bug, the border-color property is outputing values of
the properties it is a shorthand for. There is 4 of them
(top,left,right,bottom), and they are all inherit because that is what
border-color was set to...

Comment 7

[Boris Zbarsky [:bzbarsky]]

Right.  That's why I said it's not a correctness issue in comment 2.  But we
_should_ canonicalize the whole thing to a single "inherit".

Comment 8

[Hixie (not reading bugmail)]

It is a correctness issue. 'inherit inherit inherit inherit' is not a valid CSS
value for the 'border-color' property.

Comment 10

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

Attachment: work in progress

Here's some work in progress which, unfortunately, makes things worse for the direction-sensitive box properties with *-start and *-end variants to match *-right and *-left.

Comment 11

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

Attachment: patch 1: add a flags field to nsCSSPropList

I'm sure this will bitrot quickly, but it's also trivial to rewrite.  (And, thanks to mq, easy to keep separate and thus rewrite when needed.)

This may be useful for other things, too (like the always-use-my-colors pref).

Comment 12

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

Attachment: patch 2: add a flag for the *-source properties

This adds a flag for the *-{ltr,rtl}-source properties.

Comment 13

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

Attachment: patch 3: fix serialization of inherit and -moz-initial on shorthands

Comment 14

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 341975 [details] [diff] [review]
patch 3: fix serialization of inherit and -moz-initial on shorthands

Looks good, assuming that the existing code in those tests you change caught the issue with background-clip and so on.

This doesn't help GetCSSText, but that can be handled in bug 442630.

Comment 15

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

(In reply to comment #14)
> (From update of attachment 341975 [details] [diff] [review])
> Looks good, assuming that the existing code in those tests you change caught
> the issue with background-clip and so on.

Yes, the existing tests caught that (and also the issue with the *-source subproperties).

Comment 16

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

Fixed:
http://hg.mozilla.org/mozilla-central/rev/c01934ff662b
http://hg.mozilla.org/mozilla-central/rev/08d7f38b8ee3
http://hg.mozilla.org/mozilla-central/rev/7356c512e9e1