Last Comment Bug 808412 - Add "Open Calendar File…" to Calendar list context menu.
: Add "Open Calendar File…" to Calendar list context menu.
Status: RESOLVED FIXED
:
Product: Calendar
Classification: Client Software
Component: General (show other bugs)
: unspecified
: All All
: -- normal (vote)
: 1.9
Assigned To: Richard Marti (:Paenglab)
:
:
Mentors:
Depends on:
Blocks: 785480
  Show dependency treegraph
 
Reported: 2012-11-04 09:16 PST by Richard Marti (:Paenglab)
Modified: 2012-11-08 09:14 PST (History)
2 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
patch (3.12 KB, patch)
2012-11-04 09:28 PST, Richard Marti (:Paenglab)
no flags Details | Diff | Splinter Review
patch for aurora/beta (2.86 KB, patch)
2012-11-04 09:33 PST, Richard Marti (:Paenglab)
no flags Details | Diff | Splinter Review
patch for all branches (2.17 KB, patch)
2012-11-07 09:47 PST, Richard Marti (:Paenglab)
philipp: review+
philipp: approval‑calendar‑aurora+
philipp: approval‑calendar‑beta+
Details | Diff | Splinter Review

Description Richard Marti (:Paenglab) 2012-11-04 09:16:25 PST
Follow-up of bug 785480 to add the missing menu item.
Comment 1 Richard Marti (:Paenglab) 2012-11-04 09:28:22 PST
Created attachment 678136 [details] [diff] [review]
patch

Like in bug 785480 comment 9 proposed, I've added the menu item on calendar list context menu.
Comment 2 Richard Marti (:Paenglab) 2012-11-04 09:33:01 PST
Created attachment 678137 [details] [diff] [review]
patch for aurora/beta

The same patch for Aurora/Beta without localization. I've added the label/accesskey entities directly to the XUL file. If the translation still would happen, then this would be taken.
Comment 3 Stefan Sitter 2012-11-04 14:23:38 PST
> The same patch for Aurora/Beta without localization. I've added the
> label/accesskey entities directly to the XUL file. If the translation still
> would happen, then this would be taken.

Why not reuse the existing, already translated menu string instead of hard coding the English string into the xul file?
Comment 4 Richard Marti (:Paenglab) 2012-11-05 00:06:29 PST
(In reply to Stefan Sitter from comment #3)
> > The same patch for Aurora/Beta without localization. I've added the
> > label/accesskey entities directly to the XUL file. If the translation still
> > would happen, then this would be taken.
> 
> Why not reuse the existing, already translated menu string instead of hard
> coding the English string into the xul file?

Because the string is only "Calendar File…" without "Open". I don't know if it would work when I make a patchwork of the two strings "Open" and "Calendar File…". I can try this this evening.
Comment 5 Stefan Sitter 2012-11-05 02:26:10 PST
Ah, OK. I see now that "Open Calendar File…" is only defined in sunbird/menuOverlay.dtd but not in Lightning.
Comment 6 Richard Marti (:Paenglab) 2012-11-05 08:37:51 PST
I checked how the joined label would look in German: Öffnen Kalenderdatei…
This isn't good German so I think it's better leave it English than this weird German or other langage (maybe they also need to change the order).
Comment 7 Matthew Mecca [:mmecca] 2012-11-06 17:42:41 PST
We could also just use Open -> Calendar File as a sub-menu for aurora and beta. It would be inconsistent with the "Open Saved Message" menu item, but better than an untranslated string IMO.
Comment 8 Philipp Kewisch [:Fallen] 2012-11-06 23:57:32 PST
(In reply to Matthew Mecca [:mmecca] from comment #7)
> We could also just use Open -> Calendar File as a sub-menu for aurora and
> beta. It would be inconsistent with the "Open Saved Message" menu item, but
> better than an untranslated string IMO.

Yeah, I agree. Maybe modify the DOM like this:

Open >
        Calendar File
        Open Saved Message
Comment 9 Richard Marti (:Paenglab) 2012-11-07 09:47:32 PST
Created attachment 679206 [details] [diff] [review]
patch for all branches

Now on AppMenu/File with the new submenu "Open" like on main menu.

Do we want this also for trunk? Then we have not only this menu for two versions.
Comment 10 Philipp Kewisch [:Fallen] 2012-11-08 00:03:19 PST
Comment on attachment 679206 [details] [diff] [review]
patch for all branches

Review of attachment 679206 [details] [diff] [review]:
-----------------------------------------------------------------

I'm fine with taking this for all branches, r=philipp

::: calendar/lightning/content/lightning-menus.xul
@@ +452,5 @@
>                 observes="calendar_new_calendar_command"
>                 insertafter="appmenu_newAccountMenuItem"/>
>    </menupopup>
> +  
> +  <menupopup id="appmenu_FilePopup">

Extra whitespace

@@ +470,5 @@
> +                  oncommand="openLocalCalendar();"/>
> +      </menupopup>
> +    </menu>
> +  </menupopup>
> +  <menuitem id="appmenu_openMessageFileMenuitem" hidden="true"/>  

Extra whitespace

Note You need to log in before you can comment on or make changes to this bug.