Closed
Bug 415781
Opened 17 years ago
Closed 17 years ago
Page Bookmarked text on star UI is misleading
Categories
(Firefox :: Bookmarks & History, defect, P2)
Firefox
Bookmarks & History
Tracking
()
VERIFIED
FIXED
Firefox 3
People
(Reporter: stevee, Assigned: mak)
References
Details
(Keywords: late-l10n)
Attachments
(2 files, 1 obsolete file)
(deleted),
image/png
|
Details | |
(deleted),
patch
|
Details | Diff | Splinter Review |
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.
Reporter | ||
Comment 1•17 years ago
|
||
Updated•17 years ago
|
OS: Windows XP → All
Assignee | ||
Comment 2•17 years ago
|
||
maybe a title like "Edit Bookmark" or "Manage Bookmark" would be better
OS: All → Windows XP
Comment 3•17 years ago
|
||
I also thought at first that it was misleading, on GNU/Linux.
OS: Windows XP → All
Comment 4•17 years ago
|
||
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.
Comment 5•17 years ago
|
||
(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?
Reporter | ||
Comment 6•17 years ago
|
||
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.
Reporter | ||
Comment 7•17 years ago
|
||
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!
Comment 8•17 years ago
|
||
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.
Comment 10•17 years ago
|
||
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.
Updated•17 years ago
|
Assignee: nobody → mak77
Updated•17 years ago
|
Hardware: PC → All
Target Milestone: --- → Firefox 3 beta5
Assignee | ||
Comment 11•17 years ago
|
||
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)
Assignee | ||
Updated•17 years ago
|
Status: NEW → ASSIGNED
Comment 12•17 years ago
|
||
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+
Comment 13•17 years ago
|
||
(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?
Comment 14•17 years ago
|
||
(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.
Comment 15•17 years ago
|
||
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.
Comment 16•17 years ago
|
||
(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
Comment 17•17 years ago
|
||
Comment on attachment 310242 [details] [diff] [review]
patch
Like this!
Attachment #310242 -
Flags: approval1.9?
Updated•17 years ago
|
Attachment #310242 -
Flags: approval1.9? → approval1.9+
Assignee | ||
Updated•17 years ago
|
Whiteboard: [check-in after beta5 freeze]
Assignee | ||
Updated•17 years ago
|
Whiteboard: [check-in after beta5 freeze] → [check-in after beta5 freeze][needs updated patch]
Assignee | ||
Comment 19•17 years ago
|
||
for check-in after beta5
Attachment #310242 -
Attachment is obsolete: true
Assignee | ||
Updated•17 years ago
|
Whiteboard: [check-in after beta5 freeze][needs updated patch] → [check-in after beta5 freeze]
Updated•17 years ago
|
Keywords: checkin-needed
Whiteboard: [check-in after beta5 freeze]
Comment 20•17 years ago
|
||
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: 17 years ago
Resolution: --- → FIXED
Updated•17 years ago
|
Keywords: checkin-needed
Assignee | ||
Comment 21•17 years ago
|
||
posted message on mozilla.dev.l10n
Comment 22•17 years ago
|
||
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
Comment 23•17 years ago
|
||
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).
Comment 24•17 years ago
|
||
This is true, because the page _is_ actually bookmarked. See Bug 436199.
Comment 25•17 years ago
|
||
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.
Comment 26•17 years ago
|
||
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.)
Comment 27•15 years ago
|
||
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.
Description
•