716401 - Some bookmark.*Cmd are missing an ellipsis

Status: RESOLVED FIXED

(SeaMonkey :: Bookmarks & History, defect)

Description

[Serge Gautherie (:sgautherie)]

[Mozilla/5.0 (Windows NT 5.0; rv:12.0a1) Gecko/20120104 Firefox/12.0a1 SeaMonkey/2.9a1]

bookmarkLinkCmd and bookmarkPageCmd open a dialog: need ellipsis.

I didn't test bookmarkFrameCmd and bookmarkLinksCmd.

Comment 1

[Philip Chee]

Firefox doesn't use ellipsis either for these two items. Perhaps there is some method to the madness?

Comment 2

[Serge Gautherie (:sgautherie)]

(In reply to Philip Chee from comment #1)
> Firefox doesn't use ellipsis either for these two items. Perhaps there is
> some method to the madness?

I'm not sure ;-<
I didn't check Firefox. I think ellipsis should be there when there is an associated dialog.
bookmarkLinkCmd has ellipsis in mailnews and history, not in contentarea... though I didn't check how the two former actually behaves.

Comment 4

[Jens Hatlak (:InvisibleSmiley)]

(In reply to Serge Gautherie (:sgautherie) from comment #2)
> (In reply to Philip Chee from comment #1)
> > Firefox doesn't use ellipsis either for these two items. Perhaps there is
> > some method to the madness?
> 
> I'm not sure ;-<
> I didn't check Firefox.

No need to repeat mistakes (IOW: I don't care what FF does).

> I think ellipsis should be there when there is an associated dialog.

!

Reposting what I wrote in bug 716698 (duping this one against it would have been easier I guess):

As found/discussed in bug 224433, some context menu items related to bookmarking miss an ellipsis that indicates that a new window/dialog will open, i.e. the action is not instant. This observation is based on current (trunk) code, i.e. with Places-based bookmarks.

Examples of items and actions:
* Bookmark This Page -> Edit This Bookmark (doorhanger)
* Bookmark This Link -> New Bookmark (dialog)
* Bookmark This Frame -> New Bookmark (dialog)

Note: Bookmarks/Bookmark This Page is a counter-example (it *is* instant).

Entities are defined in (UTF-8) file suite/locales/en-US/chrome/common/contentAreaCommands.dtd:
* bookmarkPageCmd.label
* bookmarkLinkCmd.label
* bookmarkFrameCmd.label

Note 2: The Bookmarks menu uses addCurPageCmd.label from navigatorOverlay.dtd, so we can safely change all of the above entities.

Open question: Do we really need to rename the entities if we only add ellipses?

Comment 5

[Philip Chee]

> Open question: Do we really need to rename the entities if we only add ellipses?
No. But you should post a heads up to the L10n newsgroup.

Comment 6

[Jens Hatlak (:InvisibleSmiley)]

Attachment: patch

Comment 7

[Serge Gautherie (:sgautherie)]

(In reply to Serge Gautherie (:sgautherie) from comment #0)
> I didn't test bookmarkFrameCmd and bookmarkLinksCmd.

To be explicit, did you check bookmarkLinksCmd from history.dtd?

Comment 8

[Jens Hatlak (:InvisibleSmiley)]

(In reply to Serge Gautherie (:sgautherie) from comment #7)
> (In reply to Serge Gautherie (:sgautherie) from comment #0)
> > I didn't test bookmarkFrameCmd and bookmarkLinksCmd.
> 
> To be explicit, did you check bookmarkLinksCmd from history.dtd?

No. Seems to require an ellipsis, too (opens a New Bookmarks dialog). [STR: Open History window, select multiple page entries, context-click and select Bookmark These Links.] Let's see what Neil says in general; I can easily modify history.dtd at any time (new patch, or even upon check-in).

Comment 9

[[e-mail]]

Comment on attachment 587428 [details] [diff] [review]
patch

Not 100% sure about the Bookmark Page one since presumably you can click the arrow panel away and the bookmark will still be saved...

Comment 10

[Jens Hatlak (:InvisibleSmiley)]

Attachment: patch v1a [Checkin: Comment 11]

[now with history.dtd, see discussion with Serge]

(In reply to [e-mail] from comment #9)
> Not 100% sure about the Bookmark Page one since presumably you can click the
> arrow panel away and the bookmark will still be saved...

Well, yes, but otherwise it's pretty much an OK/Cancel situation - unlike with the equally-named Bookmarks menu item (bah, why oh why this inconsistency?).

Comment 11

[Jens Hatlak (:InvisibleSmiley)]

Comment on attachment 587498 [details] [diff] [review]
patch v1a [Checkin: Comment 11]

http://hg.mozilla.org/comm-central/rev/fad5022b0bf7