Closed Bug 415781 Opened 14 years ago Closed 14 years ago

Page Bookmarked text on star UI is misleading

Categories

(Firefox :: Bookmarks & History, defect, P2)

defect

Tracking

()

VERIFIED FIXED
Firefox 3

People

(Reporter: stevee, Assigned: mak)

References

Details

(Keywords: late-l10n)

Attachments

(2 files, 1 obsolete file)

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9b4pre) Gecko/2008020501 Minefield/3.0b4pre ID:2008020501

If you go to a page that you haven't visited before, the star is white. If you click it once, the star turns gold and you have bookmarked the page AIUI. However, if you click the gold star, you get some UI that says "Page Bookmarked" at the top.

This text is misleading to me since the page was bookmarked when the star first turned gold, but the "Page Bookmarked" makes it sound like it just got bookmarked with the second click of the star.

So really, "Page Bookmarked" should really be "This page is already bookmarked" or something. But that text is too long, so I don't really know what to suggest here instead.
OS: Windows XP → All
maybe a title like "Edit Bookmark" or "Manage Bookmark" would be better
OS: All → Windows XP
I also thought at first that it was misleading, on GNU/Linux.
OS: Windows XP → All
I believe there was a bug for a message to the user that the URL was bookmarked when he clicks the star for the first time. So people know how it works when you click it for the second time. Only, I can't find this bug; so maybe it was just a comment in a bug.
Blocks: 393509
(In reply to comment #4)
Maybe bug 415439 comment #4 and/or bug 393509 comment #108 ? Or some other bug blocking bug 393509 or blocked by it?
In fact, if you star something and then hover the gold star, the tooltip says 'Edit this bookmark' so the dialog that appears should probably say the same.
But in bug 417148 comment 1 sdwlish says "The intended behavior of the star is to have it always show up in your history, regardless of how often you visit it.  It's not meant to be a quick bookmark button..." so why does hovering the star produce a tooltip saying "Bookmark this page".

The star has an identity crisis I think!
Completely correct.  You cannot explain why clicking the star for the second time, has the tool tip of edit bookmark, yet when clicked the titled is page bookmarked.  It too, should be, Edit bookmark, for consistency.
Duplicate of this bug: 423084
The right title for the dialog, when a bookmark exists, is "Edit This Bookmark". It'll be a late-l10n hit for RC1, yes.

This will also fix bug 415439, as it will make the dialog context sensitive in that ESC/Cancel will always undo the action described by the title of the panel.
Flags: blocking-firefox3+
Keywords: late-l10n
Priority: -- → P2
Assignee: nobody → mak77
Hardware: PC → All
Target Milestone: --- → Firefox 3 beta5
Attached patch patch (obsolete) — Splinter Review
so, if i got this correctly, when we add a bookmark and immediately open the panel (like from bookmark menuitem "bookmark this page") the title is Page Bookmarked, Cancel button remove the bookmark.
when instead the page has been bookmarked before (or starred) and we click the star, the title is Edit This Bookmark and the cancel button rollback the edit.

So this should be enough, and we can take this for beta5
Attachment #310242 - Flags: review?(mano)
Status: NEW → ASSIGNED
Comment on attachment 310242 [details] [diff] [review]
patch


>-    // "Page Bookmarked" title
>+    // Set panel title:
>+    // if we are batching, i.e. the bookmark has been added now,
>+    // then show Page Bookmarked, else if the bookmark did already exist,
>+    // we are about editing it, then use Edit This Bookmark.
>     this._element("editBookmarkPanelTitle").value =
>-      bundle.getString("editBookmarkPanel.pageBookmarkedTitle");
>+      this._batching ?
>+      bundle.getString("editBookmarkPanel.pageBookmarkedTitle") :
>+      bundle.getString("editBookmarkPanel.editBookmarkTitle");

please either indent the conditional or put in a temp var.

> editBookmarkPanel.bookmarkedRemovedTitle=Bookmark Removed
>+editBookmarkPanel.editBookmarkTitle=Edit This Bookmark

please post to mozilla.dev.l10n, notifying the localizers of this late change

r=me
Attachment #310242 - Flags: review?(mano) → review+
(In reply to comment #12)
> please post to mozilla.dev.l10n, notifying the localizers of this late change
are we even allowed to do this at this point?
(In reply to comment #13)
> (In reply to comment #12)
> > please post to mozilla.dev.l10n, notifying the localizers of this late change
> are we even allowed to do this at this point?
> 

See comment #10 from Comandante Driver Beltzner.
Not allowed to check in without explicit approval, no, and at this point we'll hold off on this until after Beta 5 so that we don't break our localized builds.
(In reply to comment #15)
> Not allowed to check in without explicit approval, no, and at this point we'll
> hold off on this until after Beta 5 so that we don't break our localized
> builds.
> 

Oops, sorry. How do we ask for explicit approval for late-10n?
Target Milestone: Firefox 3 beta5 → Firefox 3
Attachment #310242 - Flags: approval1.9? → approval1.9+
Whiteboard: [check-in after beta5 freeze]
Whiteboard: [check-in after beta5 freeze] → [check-in after beta5 freeze][needs updated patch]
Duplicate of this bug: 423710
Attached patch for check-inSplinter Review
for check-in after beta5
Attachment #310242 - Attachment is obsolete: true
Whiteboard: [check-in after beta5 freeze][needs updated patch] → [check-in after beta5 freeze]
Keywords: checkin-needed
Whiteboard: [check-in after beta5 freeze]
Checking in browser/base/content/browser-places.js;
/cvsroot/mozilla/browser/base/content/browser-places.js,v  <--  browser-places.js
new revision: 1.124; previous revision: 1.123
done
Checking in browser/locales/en-US/chrome/browser/browser.properties;
/cvsroot/mozilla/browser/locales/en-US/chrome/browser/browser.properties,v  <--  browser.properties
new revision: 1.68; previous revision: 1.67
done
Status: ASSIGNED → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
posted message on mozilla.dev.l10n
Verified with:
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9pre) Gecko/20080429006
Minefield/3.0pre
and
Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.4; en-US; rv:1.9pre) Gecko/2008042904 Minefield/3.0pre
Status: RESOLVED → VERIFIED
Another reason the "Page Bookmarked" text is misleading: it appears immediately when I press Control+D, even before I choose a location for the bookmark or confirm that I want the bookmark by pressing Enter (or the "Done" button).
This is true, because the page _is_ actually bookmarked. See Bug 436199.
The interface is instant apply (why we use "Done" instead of "Ok") when you hit
control-d the bookmark immediately begins to exist in your collection of
bookmarks.  For instance, if you change the location to the bookmarks toolbar,
you will see it there while the window is still up.  We chose this UI to
streamline the interaction, and to be consistent with the behavior of clicking
the star.
Okay, it may be true, but the behavior is unlike that of any other GUI bearing a Cancel button that I have ever used.  That makes it rather counter-intuitive.  (However, I realize that this is a different problem, probably best addressed in a separate bug report.  I'll take a look at bug 436199.)
Bug 451915 - move Firefox/Places bugs to Firefox/Bookmarks and History. Remove all bugspam from this move by filtering for the string "places-to-b-and-h".

In Thunderbird 3.0b, you do that as follows:
Tools | Message Filters
Make sure the correct account is selected. Click "New"
Conditions: Body   contains   places-to-b-and-h
Change the action to "Delete Message".
Select "Manually Run" from the dropdown at the top.
Click OK.

Select the filter in the list, make sure "Inbox" is selected at the bottom, and click "Run Now". This should delete all the bugspam. You can then delete the filter.

Gerv
Component: Places → Bookmarks & History
QA Contact: places → bookmarks
You need to log in before you can comment on or make changes to this bug.