389951 - Close-button in Today-pane is checked

Status: VERIFIED FIXED

(Calendar :: Calendar Frontend, defect)

Description

[Berend Cornelius]

As soon as the todaypane has been once collapsed and expanded again the close-button in the upper right corner is checked - just like the toolbar-button in the mail toolbar. This should not be. The close button should not be checked.

Comment 1

[Berend Cornelius]

Attachment: patch to review

Please review: I resolved the issue by explicitly setting the checked attribute to false;

Comment 2

[Michael Büttner (no reviews TFN)]

Comment on attachment 274279 [details] [diff] [review]
patch to review

>+    document.getElementById("today-closer").setAttribute("checked", "false");
First, the button doesn't have an id. So calling setAttribute on above query will just throw an exception and don't do anything at all.

Second, the button is wired to the 'cmd_toggleTodayPane' command and thus inherits all attributes of this command. That's what commands are for and just works as intended. That's why I don't see a reason for manually removing the attribute.

Please state the rationale for why this is necessary, otherwise I suggest to resolve this issue as WONTFIX.

Comment 3

[Michael Büttner (no reviews TFN)]

Removing blocking flag, this clearly doesn't block the 0.7 release.

Comment 4

[Sebastian Schwieger]

Michael, I don't quite understand your rationale why you vote for WONTFIX. This is definitely a bug, I don't see any reason why the close-button should have checked state like the toolbar-button. It looks strange and is IMO wrong.

Comment 5

[Michael Büttner (no reviews TFN)]

(In reply to comment #4)
> Michael, I don't quite understand your rationale why you vote for WONTFIX. This
> is definitely a bug, I don't see any reason why the close-button should have
> checked state like the toolbar-button. It looks strange and is IMO wrong.
Uhm, maybe I'm missing the obvious, so please feel free to correct me. We're talking about a xul-attribute, not anything visible to the user. So I don't understand what's the problem?

Comment 6

[Sebastian Schwieger]

Attachment: Screenshot of close-button

It is visible. See the screenshot. After fresh start of Thunderbird, the grey background is just the same as the toolbar grey. After pressung the today-pane-toolbar-button it looks checked (lighter background).

Comment 7

[Sebastian Schwieger]

Attachment: Screenshot of close-button

It is visible. See the screenshot. After fresh start of Thunderbird, the grey background is just the same as the toolbar grey. After pressing the today-pane-toolbar-button it looks checked (lighter background).

Comment 8

[Michael Büttner (no reviews TFN)]

Admittedly I just didn't notice this as it is much much less visible on my theme compared to yours. Sorry for that. Unfortunately, I still can't r+ the patch as it is incomplete (applied as it stands it just throws an exception).

I just tracked down why this happens at all. The rule that is responsible for the lighter background is toolbarbutton[checked=true] in chrome://global/skin/toolbarbutton.css which states background-image = url(chrome://global/skin/toolbar/lighten.png). So there are 3 options available to avoid this:

1) remove the checked attribute as it is automatically set from the command
2) add a new rule setting the background explicitly
3) use a button instead of a toolbarbutton

I think option 1 is the best one, which is just what the patch does. I'm giving it r+ if the missing identifier has been corrected. Thanks to Sebastian for correcting me.

Comment 9

[Berend Cornelius]

Attachment: second patch with definition of identifier

I am sorry that the identifier definition did not make its way into the previous patch

Comment 10

[Michael Büttner (no reviews TFN)]

Comment on attachment 276474 [details] [diff] [review]
second patch with definition of identifier

looks good -> r=mickey.

Comment 11

[Michael Büttner (no reviews TFN)]

patch checked in on trunk and MOZILLA_1_8_BRANCH

-> FIXED

Berend, please always add a comment to the bug after having something checked in.

Comment 12

[Martin Schröder [:mschroeder]]

VERIFIED with Tb 2.0.0.7pre (20070814) + Lightning 0.7pre (2007081405) on WinXP.