Closed
Bug 394771
Opened 17 years ago
Closed 15 years ago
Lightning calendar view context menus use wrong entities
Categories
(Calendar :: Lightning Only, defect)
Calendar
Lightning Only
Tracking
(Not tracked)
RESOLVED
FIXED
1.0b2
People
(Reporter: rimas, Assigned: Fallen)
References
()
Details
(Whiteboard: [good first bug])
Attachments
(2 files)
10.78 KB,
patch
|
ssitter
:
review-
|
Details | Diff | Splinter Review |
10.65 KB,
patch
|
ssitter
:
review+
|
Details | Diff | Splinter Review |
Calendar context menus in Lightning currently use calendar.prev*.button.tooltip and calendar.next*.button.tooltip, but they use accesskeys from a completely different file, that are defined for different entities: goPreviousCmd.accesskey and goNextCmd.accesskey.
I think these context menus should either use their own strings, or at least use goPreviousCmd.*.label and goNextCmd.*.label, which are also used for application menus in Sunbird.
Comment 1•17 years ago
|
||
see http://mxr.mozilla.org/seamonkey/source/calendar/sunbird/base/content/calendar-menubar.inc#346 for Sunbird behavior
Comment 2•17 years ago
|
||
Rimas, you filed this as a trunk-only bug. Is this really not happening on the 1.8 branch as well?
Reporter | ||
Comment 3•17 years ago
|
||
Simon, I filed it as a trunk bug because 0.7 is trunk AFAIK, and there is no such choice as "Lightning 0.7" anyway. Actually, I've only tested it with a 1.8 build.
Comment 4•17 years ago
|
||
(In reply to comment #2)
> Rimas, you filed this as a trunk-only bug. Is this really not happening on the
> 1.8 branch as well?
Simon, the Version field is not useful for Calendar at the moment. We have options Trunk, unspecified and some released versions. What is the correct Version setting for nightlys from branch?
Comment 5•17 years ago
|
||
If a bug is reproducible on the Branch, then use "unspecified". If it is reproducible on both branch and trunk, then use "unspecified" as well.
If it is only reproducible on the trunk then use "Trunk".
Version: Trunk → unspecified
Comment 6•17 years ago
|
||
There seems to be a further superfluous entity. The entity
<!ENTITY calendar.context.deleteitem.label "Delete Item">
in calendar.dtd has an own accesskey entity:
<!ENTITY calendar.context.deleteitem.accesskey "l">
But this entity isn't used. It is used
<!ENTITY lightning.context.deleteitem.accesskey "D">
from lightning.dtd instead. What is <!ENTITY calendar.context.deleteitem.accesskey "l"> for? Is it superfluous?
Comment 7•17 years ago
|
||
To #Comment 6:
Created separate bug 396159 for that because it concerns another entity.
Comment 8•17 years ago
|
||
Does the issue still exists in recent 0.9pre nightly builds after the context menu consolidation in Bug 430430?
Comment 9•16 years ago
|
||
Rimas, can you check if this issue still exists in Lightning 1.0pre, please?
Reporter | ||
Comment 10•16 years ago
|
||
I think it's obvious from just looking at the sources:
http://hg.mozilla.org/comm-central/file/e328e7f11872/calendar/base/content/calendar-common-sets.xul#l238
Updated•15 years ago
|
Severity: normal → minor
Whiteboard: [good first bug]
Assignee | ||
Comment 11•15 years ago
|
||
This should take care
Comment 12•15 years ago
|
||
Comment on attachment 432620 [details] [diff] [review]
Fix - v1
>+++ b/calendar/base/content/calendar-common-sets.xul
>+ label-day="&calendar.prevday.label;"
>+ label-week="&calendar.prevweek.label;"
>+ label-multiweek="&calendar.prevweek.label;"
>+ label-month="&calendar.prevmonth.label;"
>+ accesskey-day="&calendar.prevday.accesskey;"
>+ accesskey-week="&calendar.prevday.accesskey;"
>+ accesskey-multiweek="&calendar.prevday.accesskey;"
>+ accesskey-month="&calendar.prevday.accesskey;"/>
Use the correct entity for prevday/prevweek/prevmonth access key
>+++ b/calendar/locales/en-US/chrome/calendar/calendar.dtd
>-<!ENTITY calendar.prevmonth.button.tooltip "Previous Month" >
>+<!ENTITY calendar.nextday.label "Next Day" >
>+<!ENTITY calendar.prevday.label "Previous Day" >
>+<!ENTITY calendar.nextday.accesskey "x" >
>+<!ENTITY calendar.prevday.accesskey "s" >
>+<!ENTITY calendar.nextweek.label "Next Week" >
>+<!ENTITY calendar.prevweek.label "Previous Week" >
>+<!ENTITY calendar.nextweek.accesskey "x" >
>+<!ENTITY calendar.prevweek.accesskey "s" >
>+<!ENTITY calendar.nextmonth.label "Next Month" >
>+<!ENTITY calendar.prevmonth.label "Previous Month" >
>+<!ENTITY calendar.nextmonth.accesskey "x" >
>+<!ENTITY calendar.prevmonth.accesskey "s" >
>
> <!ENTITY calendar.navigation.nextday.tooltip "One Day Forward" >
Please get rid of the superfluous whitespace you added.
>+++ b/calendar/sunbird/base/content/calendar-menubar.inc
>+ accesskey=""
>+ label-day="&calendar.prevday.label;"
>+ label-week="&calendar.prevweek.label;"
>+ label-multiweek="&calendar.prevweek.label;"
>+ label-month="&calendar.prevmonth.label;"
>+ accesskey-day="&calendar.prevday.accesskey;"
>+ accesskey-week="&calendar.prevday.accesskey;"
>+ accesskey-multiweek="&calendar.prevday.accesskey;"
>+ accesskey-month="&calendar.prevday.accesskey;"
> observes="calendar_view_prev_command"/>
Use the correct entity for prevday/prevweek/prevmonth access key.
> <menuitem id="calendar-go-menu-next"
> label=""
>- label-day="&goNextCmd.day.label;"
>- label-week="&goNextCmd.week.label;"
>- label-multiweek="&goNextCmd.week.label;"
>- label-month="&goNextCmd.month.label;"
>- accesskey="&goNextCmd.all.accesskey;"
>+ accesskey=""
>+ label-day="&calendar.nextday.label;"
>+ label-week="&calendar.nextweek.label;"
>+ label-multiweek="&calendar.nextweek.label;"
>+ label-month="&calendar.nextmonth.label;"
>+ accesskey-day="&calendar.nextday.accesskey;"
>+ accesskey-week="&calendar.nextweek.accesskey;"
>+ accesskey-multiweek="&calendar.nextweek.accesskey;"
>+ accesskey-month="&calendar.nextmonth.accesskey;"/>
> observes="calendar_view_next_command"/>
Close the menuitem element only once.
Attachment #432620 -
Flags: review?(ssitter) → review-
Assignee | ||
Comment 13•15 years ago
|
||
Man I wonder why the previous patch even worked! Sorry about the bad state of the patch, I hope this one does better.
Attachment #432911 -
Flags: review?(ssitter)
Comment 14•15 years ago
|
||
Comment on attachment 432911 [details] [diff] [review]
Fix - v2
r=ssitter
Nit: Should we file a followup bug to unify the wordings? For example the Go menu and the view context menu use "Next Month" but the view navigation button and the minimonth use "One Month Forward". Same for "Go to Today" vs. "Go to today".
Attachment #432911 -
Flags: review?(ssitter) → review+
Assignee | ||
Comment 15•15 years ago
|
||
Yes, I think we should. If you have a moment, feel free to file it, otherwise I'll try to do so this week.
Pushed to comm-central <http://hg.mozilla.org/comm-central/rev/98e96c27cb83>
-> FIXED
Status: ASSIGNED → RESOLVED
Closed: 15 years ago
Resolution: --- → FIXED
Target Milestone: --- → 1.0b2
You need to log in
before you can comment on or make changes to this bug.
Description
•