154755 - [FIX] Require all values in CSS RGB color rules to be of the same, valid data type

Status: VERIFIED FIXED

(Core :: CSS Parsing and Computation, defect, P2)

Description

[Greg K.]

In strict mode, only CSS numerical RGB specifications whose three values are of
the same data type should be treated as valid. That is, three numbers, or three
percentages. Mixtures of numbers and percentages should not be treated as valid CSS.

Comment 1

[Greg K.]

Moreover, invalid data types (real numbers) should not be treated as valid
values for numberical RGB specifications. For example, color: rgb(250.5, 0, 0)
is not valid CSS and should not result in red text. (David, let me know if I
should file a separate bug about this.)

Comment 2

[Boris Zbarsky [:bzbarsky]]

Attachment: Proposed patch

This fixes both issues (mixed types and rgb(255.0, 0, 0))

Comment 3

[Boris Zbarsky [:bzbarsky]]

Attachment: quirks mode testcase

Comment 4

[Boris Zbarsky [:bzbarsky]]

Attachment: Standards mode testcase

Comment 5

[Boris Zbarsky [:bzbarsky]]

I guess I'd better take this.  ;)

Comment 6

[Greg K.]

Should we just say "Expected an integer..." everywhere instead of saying
"Expected a number..."?

Comment 7

[Boris Zbarsky [:bzbarsky]]

Attachment: Fix some of that error reporting to not report the reverse of what it means. ;)

Comment 8

[Greg K.]

Do those English-language strings need to be taken out and put in some
localizable strings file?

Comment 9

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

No.  Right now CSS error reporting is a DEBUG-only feature.  I plan to make it
localizable before it is turned on in release builds (if ever).

Comment 10

[Heikki Toivonen (remove -bugzilla when emailing directly)]

I think 1.2alpha is more appropriate milestone.

Comment 11

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

Comment on attachment 89677 [details] [diff] [review]
Fix some of that error reporting to not report the reverse of what it means.  ;)

r=glazman

Comment 12

[Greg K.]

Can someone give this an sr?

Comment 13

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

Why should this be only for strict mode?  Shouldn't we do this for all modes,
and then find out if it actually causes problems (which I somewhat doubt,
actually, with the possible exception of 0 vs. % confusion).  What do WinIE and
MacIE do?

Comment 14

[Boris Zbarsky [:bzbarsky]]

MacIE and WinIE behave just like we do (get it wrong in both modes).  I would be
perfectly happy with doing this in both modes myself.  glazou?

Comment 15

[Hixie (not reading bugmail)]

Be strict in all modes. I doubt any important pages are being affected by this.

Comment 16

[Boris Zbarsky [:bzbarsky]]

Attachment: So let it be written.

Comment 17

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

Comment on attachment 97934 [details] [diff] [review]
So let it be written.

seriously, this should carry forward r= from the previous patch.
r=glazman anyway

Comment 18

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

Comment on attachment 97934 [details] [diff] [review]
So let it be written.

sr=dbaron

Comment 19

[Boris Zbarsky [:bzbarsky]]

checked in.

Comment 20

[Madhur Bhatia]

verified