63502 - Content-Style-Type doesn't handle parameters

Status: VERIFIED FIXED

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

Description

[shibaten]

<META HTTP-EQUIV="Content-Style-Type" CONTENT="text/css; charset=Shift_JIS">
makes all inline styles ignored.
Parameters should be ignored.

Trailing spaces cause the same effect.

Comment 1

[Boris Zbarsky [:bzbarsky]]

Reporter, could you by any chance provide a testcase for this?  That would be
greatly appreciated.

Comment 2

[Hixie (not reading bugmail)]

Parameters should _not_ be ignored, they should be processed. This is probably
a duplicate. See 
   http://www.bath.ac.uk/~py8ieh/internet/importtest/extra/charset.html

Comment 3

[Asa Dotzler [:asa]]

reassign.

Comment 4

[shibaten]

Attachment: specifing Content-Style-Type with correct charset parameter cause mismatching of style-type

Comment 5

[Hixie (not reading bugmail)]

Netscape's standard compliance QA team reorganised itself once again, so taking 
remaining non-tables style bugs. Sorry about the spam. I tried to get this done 
directly at the database level, but apparently that is "not easy because of the 
shadow db", "plus it screws up the audit trail", so no can do...

Comment 6

[Pierre Saslawsky]

I'm going to attach a patch.  The first part of nsCSSLoader.cpp is a fix for the 
related bug 66190.

Marc, Daniel: please review.

Comment 7

[Pierre Saslawsky]

Attachment: fix

Comment 8

[Marc Attinasi]

I really don't like the magic number '8' (strlen("charset=") and
strlen("text/css")) - I'd prefer a constant for the strings, and using strlen on
that string. But, that is a nit, take it or leave it. [s]r=attinasi

Comment 9

[karnaze (gone)]

Moving to m0.9.2

Comment 10

[Pierre Saslawsky]

fixed

Comment 11

[Hixie (not reading bugmail)]

VERIFIED that it works for style attributes using
   http://www.bath.ac.uk/~py8ieh/internet/importtest/extra/charset.html
   http://bugzilla.mozilla.org/showattachment.cgi?attach_id=21196

Comment 12

[Hixie (not reading bugmail)]

REOPENing. This has regressed. It is causing the links on 
   http://www.passionata.com/
...to be inaccessible, too, because only half the style gets applied (the style
in the style block is applied, but the style in the style attributes does not).

Tested on Netscape Commercial Branch build. If this works on the trunk, then
I apologise, feel free to reclose.

STEPS TO REPRODUCE
   http://bugzilla.mozilla.org/showattachment.cgi?attach_id=21196

ACTUAL RESULTS
   both lines unstyled

EXPECTED RESULTS
   first line should be red

Comment 13

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

Tested on TRUNK with 20010718 (linux) and 20010724 (win2k) builds : bug confirmed.

Comment 14

[Peter Van der Beken [:peterv]]

I'll take this. I tried to fix the regression (see bug 84564), but my fix was
wrong too.

Comment 15

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

side comment on existing code : "text/css" string should be an atom....

Comment 16

[Peter Van der Beken [:peterv]]

I'm attaching a patch that fixes this problem. I agree that text/css and other
mimetypes should be atoms, but I am not going to do that.

Comment 17

[Peter Van der Beken [:peterv]]

Attachment: Patch v1

Comment 18

[Peter Van der Beken [:peterv]]

Bah, wrong patch. Right one coming up. Should we try to get this on the branch
(is that still possible?).

Comment 19

[Peter Van der Beken [:peterv]]

Attachment: Patch v1 (ignore previous)

Comment 20

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

r=glazman for the following reasons :

   - we don't have a "text/css" atom for the moment so we have to create a
     string here
   - given that fact, a char[] is the fastest way
   - Marc does not want to see the constant 8 (length of string above)
   - sizeof(foo) for |char foo[]="..."| always returns length of string foo
     plus 1
   - sizeof(X)-1 is often used in our code
   - patch works (which is a minimum, right ?-) and does the right thing

Comment 21

[Marc Attinasi]

sr=attinasi for Patch V1 (the correct one). I think that the branch *and* 0.9.3
are behind us now, however...

Comment 22

[Peter Van der Beken [:peterv]]

Checked in.

Comment 23

[Madhur Bhatia]

verified