172199 - properties set to system colors return inconsistent values

Status: RESOLVED FIXED

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

Description

[Vladimir Ermakov]

If you set color property to "activeborder" color it will return "activeborder"
in dom.
If you set border-top-color property to the same color it will return "12".

Comment 1

[Vladimir Ermakov]

Attachment: testcase

Comment 2

[Boris Zbarsky [:bzbarsky]]

glazou, this is all you.  ;)

Comment 3

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

Attachment: patch v1.0

This patch (a) fixes the reported bug
	   (b) adds support for -moz-border-*-colors that were never output

reviews please ?

Comment 4

[Christopher Aillon (sabbatical, not receiving bugmail)]

Comment on attachment 103169 [details] [diff] [review]
patch v1.0

>+      if ((nsnull != theMargin) && (nsnull != theMargin->mBorderColors) &&
>+          (nsnull != theMargin->mBorderColors[index])) {

You don't really need the second level of parentheses there since != has higher
precendence than &&.  However I can see why that would not be clear, but in
this case you can even get rid of the comparisons:

    if (theMargin && theMargin->mBorderColors &&
theMargin->mBorderColors[index])

But since that seems to be the style (ick) then roll with it...

r=caillon

Comment 5

[Peter Van der Beken [:peterv]]

Comment on attachment 103169 [details] [diff] [review]
patch v1.0

sr=peterv

Comment 6

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

checked in (trunk, 1.3a)

Comment 7

[Aleksey Nogin]

This checkin have added a warning on brad TBox:

+content/html/style/src/nsCSSDeclaration.cpp:5392
+ `PRUint8 index' might be used uninitialized in this function

Comment 8

[Christopher Aillon (sabbatical, not receiving bugmail)]

That's because the compiler is somewhat stupid.

Comment 9

[Aleksey Nogin]

> That's because the compiler is somewhat stupid.

No, not in this case. In this case it's CSS_BOGUS_DEFAULT not being strong
enough - it should do "return NS_ERROR_UNEXPECTED" (or similar) instead of just
"break" - then there would be no warning...