409730 - JavaScript Debugger (Venkman) does not display the button correctly in the toolbar palette of Sunbird

Status: RESOLVED FIXED

(Other Applications Graveyard :: Venkman JS Debugger, defect)

Description

[Danilo Fortunato]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; it; rv:1.8.1.11) Gecko/20071127 Firefox/2.0.0.11
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; it; rv:1.8.1.8pre) Gecko/20071023

I installed JavaScript Debugger 0.9.87.2 on Mozilla Sunbird 0.7 (Italian locale) on Windows XP SP2, using a clean profile.

I opened the toolbar palette and I noticed that the JavaScript Debugger button was not displayed correctly (see the image attached).

This bug can be observed also on openSUSE 10.3 Linux, with the same versions of JavaScript Debugger and Mozilla Sunbird.

Reproducible: Always

Steps to Reproduce:
1. install JavaScript Debugger extension on Mozilla Sunbird 0.7, using preferably a new profile
2. right-click on the toolbar and choose the "Customize..." option
3. the toolbar palette will be shown: see the icon of the JavaScript Debugger button
Actual Results:  
The whole set of Sunbird icons is displayed.

Expected Results:  
The correct icon of JavaScript Debugger should be displayed, as it happens on Firefox.

The toolbar button is displayed correctly on Firefox 2.0.0.x.
There is no toolbar button for JavaScript Debugger available on Thunderbird 2.0.0.x.

Comment 1

[Danilo Fortunato]

Attachment: The wrong button icon of JavaScript Debugger on Mozilla Sunbird

Comment 2

[:Gijs (he/him)]

This is because when carrying over Silver's review for the patch to make venkman work on sunbird, they did change the button's id, but failed to make the stylesheet cope with this, too. There should be a toolbar button for Thunderbird fine, though - but that's a separate bug. The fix for this issue is pretty simple, I think. Patch hopefully coming up.

Comment 3

[:Gijs (he/him)]

Attachment: Patch

Comment 4

[Alex Vincent [:WeirdAl]]

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

I question this on the grounds of one-app-at-a-time.  That is, if another app wants to support Venkman in the same manner, how do we do it?

Could we reasonably change the #foo-venkman-open to .menubar-venkman-open or something?

Comment 5

[:Gijs (he/him)]

We could add a class of our own, yeah. Perhaps that's a better idea.

Comment 6

[Alex Vincent [:WeirdAl]]

Gijs, could you write up a patch for comparison?  I'm not r-'ing this one yet; I just want to see the alternate's impact.

Comment 7

[:Gijs (he/him)]

Attachment: Alternative patch using classes

This has a little bit more cost now, but less cost in the long term (as this should be the last time we'd have to touch the css file for adding support for more apps).

Comment 8

[Alex Vincent [:WeirdAl]]

Comment on attachment 295658 [details] [diff] [review]
Alternative patch using classes

Lose the "tb" part and give it something a little more generic, and r=ajvincent

Comment 9

[:Gijs (he/him)]

tb was supposed to mean "toolbar", which is pretty generic. :P

But I suppose I can lose it, and/or replace it with "toolbar"

Comment 10

[Alex Vincent [:WeirdAl]]

Oh, well, if it's toolbar, I'm fine with that.  :)

Comment 11

[:Gijs (he/him)]

Checking in mozilla/extensions/venkman/resources/content/venkman-overlay.xul;
/cvsroot/mozilla/extensions/venkman/resources/content/venkman-overlay.xul,v  <--  venkman-overlay.xul
new revision: 1.12; previous revision: 1.11
done
Checking in mozilla/extensions/venkman/resources/skin/venkman-overlay.css;
/cvsroot/mozilla/extensions/venkman/resources/skin/venkman-overlay.css,v  <--  venkman-overlay.css
new revision: 1.2; previous revision: 1.1
done

Comment 12

[Danilo Fortunato]

I tried the new JavaScript Debugger 0.9.87.3 on Mozilla Sunbird 0.7 (Italian
locale) on Windows XP SP2, using a clean profile.

The problem is still there.

Is the correction not yet available in 0.9.87.3, or the bug was not fixed properly ?
Thank you.

Comment 13

[:Gijs (he/him)]

(In reply to comment #12)
> I tried the new JavaScript Debugger 0.9.87.3 on Mozilla Sunbird 0.7 (Italian
> locale) on Windows XP SP2, using a clean profile.
> 
> The problem is still there.
> 
> Is the correction not yet available in 0.9.87.3, or the bug was not fixed
> properly ?
> Thank you.
> 

I see the button fine on the toolbar, but not in the palette (where I can repro the whole sunbird image again...). I'd guess the palette uses a different style file, somehow, but I don't have time to investigate that at the moment. Please ping in like week or something - I'm in front of an exam week. Given that it works fine on the toolbar itself, the palette is only a very minor issue, as far as I'm concerned.

Comment 14

[:Gijs (he/him)]

Huh, looks like it's actually because our style is equally general as the Sunbird one, and that gets preference (loading order? I'm not sure...). Well, looks like an !important should fix it. Or using a more specific selector. Anyone up for doing the trivial patch?