240766 - [FIX]GetAttributeDependentStyle assumes the class attribute is "class"

Status: RESOLVED FIXED

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

Description

[Boris Zbarsky [:bzbarsky]]

This is probably a safe assumption for now, but it's not that hard to fix it...

Comment 1

[Boris Zbarsky [:bzbarsky]]

Attachment: This would fix it

Comment 2

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 146331 [details] [diff] [review]
This would fix it

David, what do you think?  Note that I'm OK with just wontfixing this,
especially if we feel that it's not worth the footprint or perf (though I don't
think either is really an issue here).

Comment 3

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

Comment on attachment 146331 [details] [diff] [review]
This would fix it

The first chunk of the patch is wrong.	You can't have an if-else chain, since
you need to do all 3 parts.  (Whether you need to do both of the first two is
questionable, but you definitely need the first and third together or second
and third together.)

Aren't there other places that make similar assumptions, or is this the only
one?

Comment 4

[Boris Zbarsky [:bzbarsky]]

> The first chunk of the patch is wrong.

Indeed.  Thanks for catching that.

> Aren't there other places that make similar assumptions,

No, we've fixed the rest. This was the last one.

Comment 5

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

then this does seem worth doing

Comment 6

[Boris Zbarsky [:bzbarsky]]

Attachment: With those issues fixed

I don't see much benefit to chaining out a simple comparison there, and this
code is clearer, so I left open the possibility (admittedly remote) that ID and
class are the same attribute name (possibly in different namespaces?  OK,
remote.... ;) ).

Comment 7

[Boris Zbarsky [:bzbarsky]]

Checked in for 1.8a.