If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

Close-button in Today-pane is checked

VERIFIED FIXED in 0.7

Status

Calendar
Calendar Views
VERIFIED FIXED
10 years ago
10 years ago

People

(Reporter: Berend Cornelius, Assigned: Berend Cornelius)

Tracking

unspecified

Details

Attachments

(3 attachments, 1 obsolete attachment)

(Assignee)

Description

10 years ago
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.
Flags: blocking-calendar0.7+
(Assignee)

Comment 1

10 years ago
Created attachment 274279 [details] [diff] [review]
patch to review

Please review: I resolved the issue by explicitly setting the checked attribute to false;
Attachment #274279 - Flags: review?(michael.buettner)
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.
Attachment #274279 - Flags: review?(michael.buettner) → review-
Removing blocking flag, this clearly doesn't block the 0.7 release.
Flags: blocking-calendar0.7+ → blocking-calendar0.7-

Comment 4

10 years ago
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.
(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

10 years ago
Created attachment 275103 [details]
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

10 years ago
Created attachment 275104 [details]
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).
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.
(Assignee)

Comment 9

10 years ago
Created attachment 276474 [details] [diff] [review]
second patch with definition of identifier

I am sorry that the identifier definition did not make its way into the previous patch
Attachment #274279 - Attachment is obsolete: true
Attachment #276474 - Flags: review?(michael.buettner)

Updated

10 years ago
Status: NEW → ASSIGNED
OS: Solaris → All
Hardware: Sun → All
Version: Trunk → unspecified
Comment on attachment 276474 [details] [diff] [review]
second patch with definition of identifier

looks good -> r=mickey.
Attachment #276474 - Flags: review?(michael.buettner) → review+
(Assignee)

Updated

10 years ago
Status: ASSIGNED → RESOLVED
Last Resolved: 10 years ago
Resolution: --- → FIXED
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.
VERIFIED with Tb 2.0.0.7pre (20070814) + Lightning 0.7pre (2007081405) on WinXP.
Status: RESOLVED → VERIFIED

Updated

10 years ago
Flags: blocking-calendar0.7-
You need to log in before you can comment on or make changes to this bug.