520421 - HasAttributeDependentStyle returns false for HTML elements if the selector is not lowercase

Status: VERIFIED FIXED

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

Description

[John J. Barton]

Firebug 1.5a25 fails to work correctly on 

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.3a1pre) Gecko/20091003 Minefield/3.7a1pre

The UI does not update when it should.

1. Load http://getfirebug.com/releases/firebug/1.5X/firebug-1.5X.0a25.xpi
2. open http://getfirebug.com/tests/content/script/singleStepping/index.html
3. open firebug (status icon); reload.
4. Script Panel. Click break on next icon, (pause icon), || 
5. Click the page "click me" button.
6. The UI should should the event handler. 
   >> The execution line is not highlighted BUG
7. Move your mouse over the number "3". The highlight appears.

Case works on FF 3.5. Worked on FF 3.7 a few days ago.

Comment 1

[Boris Zbarsky [:bzbarsky]]

Can you narrow down the regression range using nightlies, if the regression is that recent?

Comment 2

[Rob Campbell [:rc] (:robcee)]

we should also test 3.6.

Comment 3

[Damon Sicore (:damons)]

blocking1.9.2+ P2.

Comment 4

[Johnathan Nightingale [:johnath]]

Adding regression-finding ninjas. Thanks, as always, for the miracles you weave.

Comment 5

[Tim Riley [:timr]]

whimboo-  can you help find the regression range?

Comment 6

[Henrik Skupin [:whimboo][⌚️UTC+1]]

As long as I can reproduce sure. Let's check.

Comment 7

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Ok, can reproduce on 3.6 too. Will start range hunting.

Comment 8

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Regressed between the builds 09062804 and 09062904:

pass: http://hg.mozilla.org/mozilla-central/rev/ca23d3b5a999
fail: http://hg.mozilla.org/mozilla-central/rev/643cdff78555

Check-ins: http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=ca23d3b5a999&tochange=643cdff78555

While digging through the list I immediately checked the HTML5 stuff and turned it off on trunk. After that everything is fine. So it would be the landing of the html5 parser which happened that day in bug 487949. CC'ing Henri.

Comment 9

[Henri Sivonen (:hsivonen)]

(In reply to comment #8)
> While digging through the list I immediately checked the HTML5 stuff and turned
> it off on trunk. After that everything is fine. So it would be the landing of
> the html5 parser which happened that day in bug 487949. CC'ing Henri.

Do you mean things are fine if you set html5.enable to false but leave the HTML5 code in the build?

Comment 10

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Yes, simply setting html5.enable to false solves this problem.

Comment 11

[Dão Gottwald [:dao]]

So this shouldn't really block.

Comment 12

[Henri Sivonen (:hsivonen)]

Most likely a dupe of bug 483015.

Comment 13

[Mike Beltzner [:beltzner, not reading bugmail]]

The HTML 5 parser is experimental for Firefox 3.6, so I agree that this shouldn't block. That said, it'd be nice for Firebug to be able to work with it on, so we should definitely try to fix it.

Comment 14

[Boris Zbarsky [:bzbarsky]]

Renominating (wish I could re-+) based on John's newsgroup comment to the effect that they don't enable the html5 parser and still see the bug....

John, can you please double-check that very carefully, just in case?

Comment 15

[Mike Beltzner [:beltzner, not reading bugmail]]

(I'll re-+ if I can get a confirmation, but for now I need to trust the word of Whimboo.)

Comment 16

[John J. Barton]

(In reply to comment #7)
> Ok, can reproduce on 3.6 too. Will start range hunting.

I tested Firebug 1.5 on 
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.2b1pre) Gecko/20091007 Namoroka/3.6b1pre

This bug does *not* occur. 

I verified that html5.enable is set to false.

Comment 17

[Boris Zbarsky [:bzbarsky]]

John, so the bug does NOT occur on 1.9.2 (with html5.enable false) but does on trunk (with html5.enable false)?

Sounds like we need a trunk regression range...

Comment 18

[John J. Barton]

I repeated the test with 
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.3a1pre) Gecko/20091008 Minefield/3.7a1pre

The bug *does* occur.

I verified that html5.enable is set to false.

Comment 19

[Boris Zbarsky [:bzbarsky]]

Whimboo, mind finding a trunk regression range?

Comment 20

[Henrik Skupin [:whimboo][⌚️UTC+1]]

John, sorry but I think I was following another bug. With HTML5 enabled and the script panel open any source/code gets never displayed. Can you reproduce this? If yes, I should file another bug.

So for the highlighting part I will start to find a regression range.

Comment 21

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Regression for this bug (no hg bisect yet) happened between the builds 090928 and 09092906:

PASS: http://hg.mozilla.org/mozilla-central/rev/e915fafc9ba5
FAIL: http://hg.mozilla.org/mozilla-central/rev/798a53f68398

Changesets: http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=e915fafc9ba5&tochange=798a53f68398

Eventually the patch on bug 507592? If someone has an idea please speak up otherwise I will have to bisect tomorrow or apply the patch on bug 507592 to test it on 1.9.2.

Comment 22

[John J. Barton]

(In reply to comment #20)
> John, sorry but I think I was following another bug. With HTML5 enabled and the
> script panel open any source/code gets never displayed. Can you reproduce this?

yes:
 Bug 521345 -  [html5] extraneous tags created

Comment 23

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Sorry for the German output but that's the first bad revision:

Die erste schlechte Revision ist:
Änderung:        33262:2c6aeb49c93c
Nutzer:          David Zbarsky <[e-mail]>
Datum:           Mon Sep 28 23:07:45 2009 -0700
Zusammenfassung: Bug 507762:  Parse CSS style sheets independently of case-sensitivity, and instead check case correctly when using the stylesheets.  r=dbaron

Comment 24

[Boris Zbarsky [:bzbarsky]]

Oh, interesting.  That bug made it so that case is correctly taken into account when matching XUL elements (or other non-HTML elements) in an HTML document.

Need to figure out whether the patch is buggy or whether the stylesheets involved were depending on the wrong behavior.  John, robcee, any idea what rules might have stopped or started matching here?

Comment 25

[John J. Barton]

(In reply to comment #24)
>... John, robcee, any idea what
> rules might have stopped or started matching here?

Bug 507762 does not have enough information for me to figure out what you are
asking.

Comment 26

[Henrik Skupin [:whimboo][⌚️UTC+1]]

John, I believe Boris wants to know how you highlight the line of code in the script panel of Firebug. Is it done via CSS? Which node is affected? Can you point us to the source of the appropriate xul file?

Comment 27

[Boris Zbarsky [:bzbarsky]]

And the appropriate CSS file.

The basic change in bug 507762 is that if your CSS says "foobar" and your XUL node is named "fooBar" then it would match before that change and no longer matches now.

Comment 28

[John J. Barton]

The highlight is set by adding an attribute: 

http://code.google.com/p/fbug/source/browse/branches/firebug1.5/content/firebug/debugger.js#2204

which triggers this rule:

http://code.google.com/p/fbug/source/browse/branches/firebug1.5/skin/classic/debugger.css#88

Comment 29

[John J. Barton]

When I inspect the Firebug element that does not get the highlight using Chromebug, the attribute is lower case: exeline="true".

When put the mouse over the line number the class "hovered" is added to the element and the highlight shows up. It remains up after "hovered" is removed. 

https://developer.mozilla.org/en/setAttribute:
    When called on an HTML element in an HTML document, 
    setAttribute lower-cases its its attribute name argument.

Comment 30

[Boris Zbarsky [:bzbarsky]]

Aha.  Definitely a regression from bug 507762.  HasAttributeDependentStyle is lying in this case.

Comment 31

[John J. Barton]

I changed all instances of "exeLine" in firebug to "exe_line" (so case insensitive) and the execution line highlight works on FF 3.7 nightly build.

Comment 32

[Boris Zbarsky [:bzbarsky]]

Yeah, the problem is cased attribute selectors applied to HTML elements.  Patch coming up in a jiffy.  Just need to finish linking layout and testing.

Comment 33

[Boris Zbarsky [:bzbarsky]]

Attachment: Patch

Comment 34

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

Comment on attachment 405506 [details] [diff] [review]
Patch

r=dbaron

Comment 36

[Boris Zbarsky [:bzbarsky]]

Pushed http://hg.mozilla.org/mozilla-central/rev/9643f825fc9b

Comment 37

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Verified fixed with Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.3a1pre) Gecko/20091012 Minefield/3.7a1pre ID:20091012135900