Editing tags/keywords in bookmarks

Bug #237679 reported by Mike Rushton
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
Mozilla Firefox
Invalid
Medium
firefox-3.0 (Ubuntu)
Confirmed
Undecided
Unassigned

Bug Description

Binary package hint: firefox-3.0

Ubuntu 8.04.1
Firefox 3.0 RC1

If you edit a bookmark by using the star in the address bar, you can edit the tags but you can't edit the keyword for it. If you edit the bookmark by right-clicking on it in the bookmarks menu and going to properties, you can edit the keyword, but not the tags.

Please add the ability to edit tags AND keywords from both of these actions.

ProblemType: Bug
Architecture: i386
Date: Thu Jun 5 12:37:53 2008
DistroRelease: Ubuntu 8.04
NonfreeKernelModules: nvidia
Package: firefox-3.0 3.0~rc1+nobinonly-0ubuntu0.8.04.1
PackageArchitecture: i386
ProcEnviron:
 PATH=/home/username/bin:/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin:/usr/games
 LANG=en_US.UTF-8
 SHELL=/bin/bash
SourcePackage: firefox-3.0
Uname: Linux 2.6.24-18-generic i686

Tags: apport-bug
Revision history for this message
In , John-virginiaquilter (john-virginiaquilter) wrote :

I realized this is better presented as an enhancement bug. Retitling to suit.

Revision history for this message
In , John-virginiaquilter (john-virginiaquilter) wrote :

No change as of 1.6.

Revision history for this message
In , Guanxi-i (guanxi-i) wrote :

Cross-references for integrating autocomplete and bookmarks:

Include in the Autocomplete list ...

Bug 101642 - Bookmark URLs, and list them on top
Bug 117967 - Bookmark names/titles
Bug 155320 - Bookmarks user clicks on
Bug 212605 - Bookmark Keywords, and don't prepend previously
      typed keywords with 'http://'

Revision history for this message
In , Guanxi-i (guanxi-i) wrote :

Confirming.

Hardware: PC -> All

Revision history for this message
In , GD (gmarketer) wrote :

Please implement this easy, but useful feature. Also, please display on the right side description of keyword in the address bar.

Revision history for this message
In , Hughes-matt (hughes-matt) wrote :

User-Agent: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9b1) Gecko/2007110903 Firefox/3.0b1
Build Identifier: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9b1) Gecko/2007110903 Firefox/3.0b1

Currently the only way to assign bookmark keywords is to add a bookmark (CNTL-D), go into the bookmark manager, find the bookmark you JUST ADDED, click on 'Properties', then click the 'More' button, and finally enter your keyword.

You should be able to assign keywords directly from the quick add dialog. It is a very small dialog already, so I can't imagine size is an issue.

I really like all the work that has been done on bookmarks in this release (Firefox 3, Beta 1) but why, oh why are bookmark keywords so shunned! They're an awesome feature, but hardly anybody knows about them because you don't even see them unless you venture into bookmark manager.

Reproducible: Always

Steps to Reproduce:
1. Go to a website
2. Add a bookmark (CNTRL-D)
3. There is no way to assign a keyword from the dialog
Actual Results:
Added a bookmark without any keyword

Expected Results:
I expect the keyword field to be on the dialog

Proof of fans of this feature: http://lifehacker.com/software/bookmarks/hack-attack-firefox-and-the-art-of-keyword-bookmarking-196779.php

Also, I believe keyword autocomplete (https://bugzilla.mozilla.org/show_bug.cgi?id=212605) should be a very nice addition to fixing this bug.

Revision history for this message
In , Hskupin (hskupin) wrote :

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

When you want to modify the bookmark properties within the bookmarks sidebar you are not able to add/remove tags because no UI exists yet. Fields (like the details deck inside the Library) should be added to be able to modify the tags for that bookmark.

Revision history for this message
In , Andrewm715+bugzilla (andrewm715+bugzilla) wrote :

This also applies for bookmarks on the bookmarks toolbar.

Revision history for this message
In , Mike-lierman-yahoo (mike-lierman-yahoo) wrote :

As far as I know this has been implemented in Firefox 3.0 Beta 2. I don't know what version of Firefox you are using but in mine I am able to tag bookmarks and then search using those tags. This works with the toolbar too.

RESOLVED?

Revision history for this message
In , Hskupin (hskupin) wrote :

This bug isn't solved yet. The Bookmark Properties dialog still lacks the tagging UI. No idea about what specific bookmarks properties you are talking about but doing the steps from my initially comment should show this for you.

Revision history for this message
In , Henryb (henryb) wrote :

Can't find a good dupe for lack of keywords in new star Bookmark Dialog, marking NEW and duping newer bug to this. Not sure how much love this is gonna get, though - that would be a big change this late in the game, but I agree that it is troublesome and user-unfriendly.

Revision history for this message
In , Henryb (henryb) wrote :

*** Bug 405795 has been marked as a duplicate of this bug. ***

Revision history for this message
In , Philringnalda (philringnalda) wrote :

*** Bug 416008 has been marked as a duplicate of this bug. ***

Revision history for this message
In , Philringnalda (philringnalda) wrote :

*** Bug 417304 has been marked as a duplicate of this bug. ***

Revision history for this message
In , Ria-klaassen (ria-klaassen) wrote :

*** Bug 419325 has been marked as a duplicate of this bug. ***

Revision history for this message
In , Cwwmozilla (cwwmozilla) wrote :

*** Bug 421568 has been marked as a duplicate of this bug. ***

Revision history for this message
In , Jo-hermans (jo-hermans) wrote :

*** Bug 422821 has been marked as a duplicate of this bug. ***

Revision history for this message
In , Mike Connor (mconnor) wrote :

Not blocking on this bug for final ship. Would take a safe enough patch if one comes through.

Revision history for this message
In , Jan Darmochwal (jdarmochwal) wrote :

*** Bug 427569 has been marked as a duplicate of this bug. ***

Revision history for this message
In , Mozilla-nuss (mozilla-nuss) wrote :

*** Bug 434238 has been marked as a duplicate of this bug. ***

Revision history for this message
In , Mozilla-nuss (mozilla-nuss) wrote :

I think there are 3 options to solve this. When clicking on Properties in the context menu in the Bookmarks sidebar:
1. Open the Library with the bookmark in question selected.
2. Open the star/"Edit this bookmark" dialog that drops down from the
AwsomeBar.
3. Just include tags in the bookmark Properties modal.

Considering that the main menu Bookmarks > Bookmark This Page does 2 (above) that seems like a nice solution. But since the Bookmarks sidebar is in a sense a light version of the Library, 1 (above) makes more sense. 3 (above), I think is the worst solution since there really is no need for a third way to edit a bookmark.

Revision history for this message
In , Jmjeffery (jmjeffery) wrote :

*** Bug 437399 has been marked as a duplicate of this bug. ***

Revision history for this message
In , Alice0775 (alice0775) wrote :

I wrote a mall extension which open "Edit This Bookmark" solution of No.2(Comment #12)
https://addons.mozilla.org/ja/firefox/addon/7396

Revision history for this message
In , Alice0775 (alice0775) wrote :

s/mall/small/

Revision history for this message
Mike Rushton (leftyfb) wrote :

Binary package hint: firefox-3.0

Ubuntu 8.04.1
Firefox 3.0 RC1

If you edit a bookmark by using the star in the address bar, you can edit the tags but you can't edit the keyword for it. If you edit the bookmark by right-clicking on it in the bookmarks menu and going to properties, you can edit the keyword, but not the tags.

Please add the ability to edit tags AND keywords from both of these actions.

ProblemType: Bug
Architecture: i386
Date: Thu Jun 5 12:37:53 2008
DistroRelease: Ubuntu 8.04
NonfreeKernelModules: nvidia
Package: firefox-3.0 3.0~rc1+nobinonly-0ubuntu0.8.04.1
PackageArchitecture: i386
ProcEnviron:
 PATH=/home/username/bin:/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin:/usr/games
 LANG=en_US.UTF-8
 SHELL=/bin/bash
SourcePackage: firefox-3.0
Uname: Linux 2.6.24-18-generic i686

Revision history for this message
Mike Rushton (leftyfb) wrote :
Revision history for this message
Alexander Sack (asac) wrote : Re: [Bug 237679] [NEW] Editing tags/keywords in bookmarks

On Thu, Jun 05, 2008 at 04:40:57PM -0000, Mike Rushton wrote:
> Public bug reported:
>
> Binary package hint: firefox-3.0
>
> Ubuntu 8.04.1
> Firefox 3.0 RC1
>
> If you edit a bookmark by using the star in the address bar, you can
> edit the tags but you can't edit the keyword for it. If you edit the
> bookmark by right-clicking on it in the bookmarks menu and going to
> properties, you can edit the keyword, but not the tags.
>
> Please add the ability to edit tags AND keywords from both of these
> actions.

Please file bugs for enhancement requests directly upstream in
bugzilla.mozilla.org. As a hint: take care which component you file it
against. If you file it against "General" nothing will happen most
likely ... remember to let us know about the bug id there.

 status incomplete

 - Alexander

Changed in firefox-3.0:
status: New → Incomplete
Revision history for this message
In , Ria-klaassen (ria-klaassen) wrote :

*** Bug 437508 has been marked as a duplicate of this bug. ***

Revision history for this message
In , Hskupin (hskupin) wrote :

mconnor, which plans are out there for the old bookmarks properties dialog? Should it be replaced by the page bookmarked panel like it was done by Alice's extension?

Revision history for this message
In , Ria-klaassen (ria-klaassen) wrote :

*** Bug 437991 has been marked as a duplicate of this bug. ***

Revision history for this message
In , Dietrich-mozilla (dietrich-mozilla) wrote :

(In reply to comment #16)
> mconnor, which plans are out there for the old bookmarks properties dialog?
> Should it be replaced by the page bookmarked panel like it was done by Alice's
> extension?
>

Yes, it should. Making this bug target 3.1.

Revision history for this message
In , Phil-rebee (phil-rebee) wrote :

Fixed by the Ex Bookmark Properties addon.

See also bug 426674

Revision history for this message
In , Phil-rebee (phil-rebee) wrote :

oops, forgot link:

Ex Bookmark Properties Addon:

https://addons.mozilla.org/en-US/firefox/addon/7396

Revision history for this message
In , Andrew-neocodenetworks (andrew-neocodenetworks) wrote :

Created an attachment (id=325882)
Adds tagging to Bookmark Properties dialog

I've made a patch to add a tag field into the Bookmark Properties dialog. The functionality was taken from the new add/edit bookmark overlay attached to the Location Bar.

Revision history for this message
In , Philringnalda (philringnalda) wrote :

*** Bug 441687 has been marked as a duplicate of this bug. ***

Revision history for this message
In , Bugzilla2007 (bugzilla2007) wrote :

In support of this enhancement: When I type a bookmark keyword [not tag] into location bar, then press enter, the URL of that keyword bookmark will definitely be loaded. It is weird that the actual URL about to be loaded is NOT shown in location bar dropdown list. But it's not the only thing that is not shown in the awesome bar dropdown though it should be... (bug 424797). The latter bug prevents effective use of the workaround for this bug 212605, namely that you can include the keyword in the title of your bookmark (e.g. "[wp] wikipedia quicksearch").

Revision history for this message
In , Xtc4uall (xtc4uall) wrote :

Andrew, i tested your patch with Minefield/3.1b1pre ID:20080902033133 and it seems to be okay (tag creation, deletion). you may want to ask for (ui-)review ?!?

Revision history for this message
In , Andrew-neocodenetworks (andrew-neocodenetworks) wrote :

(From update of attachment 325882)
Thanks, XtC4Uall. I've added the reviewers based on mconnor's suggestion from #developers.

Revision history for this message
In , Dietrich-mozilla (dietrich-mozilla) wrote :

Thanks for the patch Andrew, but I don't think we should be extending that dialog. Instead, we should be standardizing on the new bookmark properties panel, and removing the old one.

I think that the dialog should embed the new panel, like the library does. This will reduce code and provide a consistent experience for users.

I also don't like that there are two versions of bookmark properties UI available via primary UI that are so radically different in appearance, but that's an issue for another bug.

Revision history for this message
In , Xtc4uall (xtc4uall) wrote :

(In reply to comment #25)
> ... we should be standardizing on the new bookmark properties
> panel, and removing the old one.

> I think that the dialog should embed the new panel, like the library does.

anyone mind enlighten me about the "new" bookmark properties panel (mockup, bugreport, etc.)?

> I also don't like that there are two versions of bookmark properties UI
> available via primary UI that are so radically different in appearance, but
> that's an issue for another bug.

are there any bulletproof plans in 3.1 timeframe for this?
if not, i'd say get this in before there's nothing at all.

Revision history for this message
Mackenzie Morgan (maco.m) wrote :

I have opened a bug report upstream: https://bugzilla.mozilla.org/show_bug.cgi?id=454726

Changed in firefox-3.0:
status: Incomplete → Confirmed
Revision history for this message
Mike Rushton (leftyfb) wrote :

I had already reported to upstream here: https://bugzilla.mozilla.org/show_bug.cgi?id=437508

But it was marked as a duplicate of https://bugzilla.mozilla.org/show_bug.cgi?id=404507.

It looks like they have since placed adding tags to the bookmark properties dialog from the bookmark management window. But keywords is still missing from the start/quick bookmark dialog.

Revision history for this message
In , Ria-klaassen (ria-klaassen) wrote :

*** Bug 454726 has been marked as a duplicate of this bug. ***

Changed in firefox:
status: Unknown → Invalid
Revision history for this message
C de-Avillez (hggdh2) wrote :

Adjusted upstream watch.

Changed in firefox:
status: Invalid → Unknown
Changed in firefox:
status: Unknown → Confirmed
Revision history for this message
Mackenzie Morgan (maco.m) wrote :

This should also be linked against https://bugzilla.mozilla.org/show_bug.cgi?id=411261 but Launchpad won't let me do it.

Revision history for this message
In , Mak77 (mak77) wrote :

Created an attachment (id=341452)
really wip

a first take on this, really wip, edit works, add is correct but does not work (code missing for now), and this still needs A LOT of cleanup. The biggest issue is that editBookmarkOverlay does not support creation, but with some fix here and there it should work, so we would have tag autocomplete and multiedit for free.

Revision history for this message
In , Mak77 (mak77) wrote :

(In reply to comment #26)
> (In reply to comment #25)
> > ... we should be standardizing on the new bookmark properties
> > panel, and removing the old one.
>
> > I think that the dialog should embed the new panel, like the library does.
>
> anyone mind enlighten me about the "new" bookmark properties panel (mockup,
> bugreport, etc.)?

i think he's talking about the editItemOverlay that is used in the star panel and in the Library... we can embed it into the dialog, so we have only one code to manage for all the panels...
My patch tries to make that, solving the problem with the "auto apply" behaviour that is not correct for an OK,CANCEL dialog, and the creation case. It takes away a lot of duplicated code but it's still quite dirt, since that code has to manage many different cases. Ideally we should try to unify all panels under a common code, so adding a feature to that code would add to all panels (see tag autocomplete, multiple edit and so on).

> > I also don't like that there are two versions of bookmark properties UI
> > available via primary UI that are so radically different in appearance, but
> > that's an issue for another bug.

Dietrich this is something we should do later imho, before we should try to use the overlayCode in all dialogs, and then try to unify the "styling", hwv i think that having a dialog style panel is not so bad, especially on Windows, the star panel as it is (borderless and opened under the locationbar) is good until is used from the star of from Bookmark this page menuitem.

Revision history for this message
In , Mak77 (mak77) wrote :

Created an attachment (id=341621)
wip 0.2

i've tried to reduce a bit the patch moving functions to their original position and restoring some cleanup change that is unneeded actually, can be done later.

this works for both edit and add, and should provide dialogs quite similar to previous ones (so this will mostly not need an UI review since the only visible change will be the added tags field that is wanted and a more consistent disposition of elements like in other panels).
The only issue i'm aware of actually is the tags selector that is not in sync when adding a new element, will fix next wip.

taking for now since this appear working

Revision history for this message
In , Mak77 (mak77) wrote :

and "new folder" does not work correctly for now.

Revision history for this message
In , Mak77 (mak77) wrote :

Created an attachment (id=341704)
wip 0.3

fixed previously reported bugs.
todo: fill up hidden elements, support last used folder, global testing, cleanup
reminder: followup to support tag multiediting when doing bookmark all tabs

Revision history for this message
In , Hskupin (hskupin) wrote :

Marco, that's great. When your patch will be ready for testing just create a try server build and we will have a test-run.

Revision history for this message
In , Mak77 (mak77) wrote :

a panels list for future testing, this is where we are using the bookmark properties panel:

1. history sidebar, right click and choose Bookmark this link, should show name (filled) and folder picker

2. places views context menu New Bookmark, should show name, location, tags, keyword, description, loadinsidebar

3. places views context menu New Folder, should show name(filled), description

4. go to a livemark, subscribe using live bookmarks, should show name (filled) and folder picker

5. right click on a search field, Add keyword for this search, should show name, keyword, folder picker

6. add a sidebar panel on http://www.google.com/mozilla/google-search.html, should show name (filled) and folder picker

7. click on a panel add link with rel="sidebar" (tested on http://tntluoma.com/sidebars/ Add links) should show name (filled) and folder picker

8. add bookmarks button through customize toolbars, drag & drop a link upon it, should show name (filled) and folder picker

9. right click a bookmark/folder/livemark and choose properties, should show correct editable fields based on item type, with correct content

clearly all panels should work saving correct data

Revision history for this message
In , Mak77 (mak77) wrote :

10. open 2 tabs, from bookmarks menu choose Bookmark all tabs, should show name (filled) and folder picker

Revision history for this message
In , Mak77 (mak77) wrote :

filed enh request bug 458698 - when doing Bookmark All Tabs allow for tags
multiediting

Revision history for this message
In , Mak77 (mak77) wrote :

for tests 6. and 7. the bookmark should open in sidebar when clicked and when editing load in sidebar should be checked

Revision history for this message
In , Mak77 (mak77) wrote :

Created an attachment (id=341937)
patch v1

asking a first-pass review to start cleaning-up this thing.

I've done the above tests and they appear correct, however after this review i'll generate a trybuild for QA testing.

Revision history for this message
In , Dietrich-mozilla (dietrich-mozilla) wrote :
Download full text (7.0 KiB)

(From update of attachment 341937)

> * item.
> * @ defaultInsertionPoint (InsertionPoint JS object) - optional, the
> * default insertion point for the new item.
> * @ keyword (String) - optional, the default keyword for the new item.
> * @ postData (String) - optional, POST data to accompany the keyword.
>+* @ charSet (String) - optional, character-set to accompany the keyword.

nit: indent

> if ("title" in dialogInfo)
>- this._itemTitle = dialogInfo.title;
>+ this._title = dialogInfo.title;
> if ("defaultInsertionPoint" in dialogInfo)
> this._defaultInsertionPoint = dialogInfo.defaultInsertionPoint;
> else {
> // default to the bookmarks root

nit: please update this comment while you're there

> this._defaultInsertionPoint =
>- new InsertionPoint(PlacesUtils.bookmarksMenuFolderId, -1);
>+ new InsertionPoint(PlacesUtils.unfiledBookmarksFolderId, -1,
>+ Ci.nsITreeView.DROP_ON);
> }

why this change? this would silently change behavior, acting against user expectations.

>+ // Listen on uri fields to enable accept button if input is valid
>+ var inputListener = {
>+ _self: this,
>+ handleEvent: function(event) {
>+ document.documentElement.getButton("accept").disabled =
>+ !this._self._inputIsValid();
>+ }
>+ };
>+ if (this._itemType == BOOKMARK_ITEM) {
>+ this._element("locationField")
>+ .addEventListener("input", inputListener, false);
>+ }
>+ if (this._itemType == LIVEMARK_CONTAINER) {

nit: else?

> _showHideRows: function EIO__showHideRows() {
> var isBookmark = this._itemId != -1 &&
> this._itemType == Ci.nsINavBookmarksService.TYPE_BOOKMARK;
> var isQuery = false;
> if (this._uri)
> isQuery = this._uri.schemeIs("place");
>
>- this._element("nameRow").collapsed = this._hiddenRows.indexOf("name") != -1;
>+ // in insertMode hiddenRows is the only valid condition
>+ this._element("nameRow").collapsed =
>+ this._hiddenRows.indexOf("name") != -1;
> this._element("folderRow").collapsed =
>+ this._insertMode ? this._hiddenRows.indexOf("folderPicker") != -1 :
> this._hiddenRows.indexOf("folderPicker") != -1 || this._readOnly;
>-
>- this._element("tagsRow").collapsed = !this._uri ||
>- this._hiddenRows.indexOf("tags") != -1 || isQuery;
>+ this._element("tagsRow").collapsed =
>+ this._insertMode ? this._hiddenRows.indexOf("tags") != -1 :
>+ this._hiddenRows.indexOf("tags") != -1 || !this._uri || isQuery;
> this._element("descriptionRow").collapsed =
>+ this._insertMode ? this._hiddenRows.indexOf("description") != -1 :
> this._hiddenRows.indexOf("description") != -1 || this._readOnly;
>- this._element("keywordRow").collapsed = !isBookmark || this._readOnly ||
>- this._hiddenRows.indexOf("keyword") != -1 || isQuery;
>- this._element("locationRow").collapsed = !isBookmark || isQuery ||
>- this._hiddenRows.indexOf("location") != -1;
>- this._element("loadInSidebarCheckbox").collapsed = !isBookmark || isQuery ||...

Read more...

Revision history for this message
In , Mak77 (mak77) wrote :

(In reply to comment #38)
> (From update of attachment 341937 [details])
> >@@ -491,32 +491,32 @@ var PlacesUIUtils = {
> > showMinimalAddBookmarkUI:
> > function PU_showMinimalAddBookmarkUI(aURI, aTitle, aDescription,
> > aDefaultInsertionPoint, aShowPicker,
> > aLoadInSidebar, aKeyword, aPostData,
> > aCharSet) {
> > var info = {
> > action: "add",
> > type: "bookmark",
> >- hiddenRows: ["location", "description", "loadInSidebar"]
> >+ hiddenRows: ["location", "tags", "description", "loadInSidebar"]
> > };
>
> hm, i'd certainly like to be able to add tags there...

for now i want to obtain dialog parity with actual version, adding tags field to additional dialogs (for exaple minimal UI ones) will be easy to do in tiny followups.

Revision history for this message
In , Mak77 (mak77) wrote :

Created an attachment (id=342351)
patch v1.1

addressed dietrich's comments, passing to Mano since editBookmarkOverlay is mostly his work.

Revision history for this message
In , Faaborg (faaborg) wrote :
Download full text (3.1 KiB)

Created an attachment (id=343138)
This is pretty strange

In https://bugzilla.mozilla.org/show_bug.cgi?id=411261#c25 Dietrich wrote:

>Thanks for the patch Andrew, but I don't think we should be extending that
>dialog. Instead, we should be standardizing on the new bookmark properties
>panel, and removing the old one.
>
>I think that the dialog should embed the new panel, like the library does. This
>will reduce code and provide a consistent experience for users.
>
>I also don't like that there are two versions of bookmark properties UI
>available via primary UI that are so radically different in appearance, but
>that's an issue for another bug.

To review all of the motivations here:
1) reuse code to reduce complexity
2) use a consistent interface so that users are not confused

I think everyone is on board with both of those motivations, but I'm not sure how far we go with matching the bookmark contextual dialog (the thing that hangs off of the star in the location bar) in the other instances of the bookmark properties dialog. Here is the list Marco gave me on irc:

1. history sidebar, right click and choose Bookmark this link
2. places views (menu, toolbar) context menu New Bookmark, New Folder
3. subscribe a livemark
4. Add keyword for this search
5. add a sidebar panel clicking on a link or through js function addPanel
6. drop a bookmark over the bookmarks Library icon after customize toolbars
7. Bookmark all tabs
8. Context menu properties

Also, I think:
9. Library window properties pane (?)
10. "Bookmark this Link" on the contextual menu of a hyperlink in the content area (screenshot in attachment).

There might be more instances of creating / editing a bookmarks properties, but those are all of the ones I know about.

In terms of Dietrich's comment:

>I also don't like that there are two versions of bookmark properties UI
>available via primary UI that are so radically different in appearance

The bookmark contextual dialog is instant apply, and has a click outside to dismiss behavior (the click outside to dismiss behavior is conveyed visually by referencing other interfaces that are click outside to dismiss).

Adapting the instant apply nature of the interface to other instances of the bookmarks property dialog is fine, but carrying over click outside to dismiss behavior doesn't make any sense in the other contexts. For instance, the "Bookmark this Link" interface isn't visually associated with anything that could recreate the interface (like the star in the location bar) and it is kind of awkward in the upper left hand corner of the content area.

So I propose that we leverage some of the existing code, which means that they will become instant apply. However, they should really remain (including the "Bookmark this Link" instance) real modal dialog boxes, with Done / Cancel buttons.

Also, the large star icon, title and "Remove bookmark" items don't need to appear in these other instances of the bookmarks property interfaces, since a contextual command already set the context of what the user is doing, and there was also a contextual delete command.

I probably still need to clarify a bunch of stuff, but what do people think of ...

Read more...

Revision history for this message
In , Faaborg (faaborg) wrote :

>I also don't like that there are two versions of bookmark properties UI
>available via primary UI that are so radically different in appearance, but
>that's an issue for another bug.

I've filed follow up bug 459958 to determine what we should do in all of the different instances of the bookmarks property dialog.

Revision history for this message
In , Mak77 (mak77) wrote :

imho the hardest part is is mixing up in a sane way the need for a common UI with the user need of a dialog-like panel (as you said for example screen reader users, but i can tell for sure mostly Windows users prefer a classic panel to instant apply). I agree with the idea of a MODAL dialog with a border and Done/Cancel buttons so the user has to take a decision.

Instant apply is quite easy and straight-forward for the edit case (so "properties"), it's already used when clicking the active star and in the Library pane. In the add case we will have to guess some data.

Case 2 is maybe the most strange since we don't know anything about the bookmark (but probably the position where the user wants to create it, so we should create an about:blank bookmark with title New Bookmark, and let the user edit it, but we should present empty fields that are easier to fill).

Revision history for this message
In , Hskupin (hskupin) wrote :

Alex, one thing I noticed some days back is the complicated way how to chance the URL of an already saved bookmark. Currently we aren't able to do this via the contextual dialog. Users have to open the Library, navigating to the folder the bookmark resists in or doing a search within the Library. Afterwards they can change the URL in the details pane. But this is the only place. IMO this simple action is really complicated and needs a lot of user interaction until the aim can be achieved.

At least the bookmark properties dialog shouldn't miss this field. It will make the life a lot easier for users who only want to update an outdated URL without the need of removing, re-adding, and retagging.

Revision history for this message
In , Faaborg (faaborg) wrote :

This because we are thinking about bookmarking being more like flagging an email, just a quick gesture to mark something interesting. If someone wants to change the URL of a bookmark, I would argue that what they really want to do is bookmark something else (just like you don't change the email of an existing flagged email, you flag some other email). Changing the folder, set of tags and name is extra work, but urls don't change all that often, and someone who sets all of this information will probably be familiar with the library window or sidebar to select "properties."

Revision history for this message
In , Mak77 (mak77) wrote :
Download full text (3.2 KiB)

Alex any news here?

I have a first working implementation (instant apply in a dialog) with following situation, hope this helps you telling what should be hidden and what should be changed here, after your first suggestions i would create a trybuild so you could suggest further refinements:

1. in history sidebar, right click on a visit and choose Bookmark this link
   Contents: name(filed), folder picker(bookmarks menu), Tags(empty)
   Title: "Add Bookmark"
   AcceptButton: "Add"

2. on Places views (menus, toolbars, trees) right click and choose New Bookmark
   contents: name("New Bookmark"), location("about:blank"), tags(empty), keyword(empty), description(empty), load in sidebar(unchecked)
   Title: "Add Bookmark"
   AcceptButton: "Add"

3. on Places views (menus, toolbars, trees) right click and choose New Folder
   contents: name("New Folder"), description(empty)
   Title: "Add Folder"
   AcceptButton: "Add"

4. go to a livemark, click subscribe using live bookmarks
   contents: name(filed), folder(bookmarks toolbar)
   Title: "Add Live Bookmark"
   AcceptButton: "Add"

5. right click on a search field, "Add keyword for this search"
   name, keyword, folder picker
   contents: name(empty), folder(bookmarks menu), tags(empty), keywords(empty)
   Title: "Add Bookmark"
   AcceptButton: "Add"

6. add a sidebar panel on http://www.google.com/mozilla/google-search.html
   contents: name(filed), folder(bookmarks menu), tags(empty)
   Title: "Add Bookmark"
   AcceptButton: "Add"

7. click on a panel Add link with rel="sidebar" (tested on
http://tntluoma.com/sidebars/ Add links)
   contents: name(filed), folder(bookmarks menu), Tags(empty)
   Title: "Add Bookmark"
   AcceptButton: "Add"

8. add bookmarks button through customize toolbars, drag & drop a link upon it
should show name (filled) and folder picker
   contents: name(filed), folder(bookmarks menu), Tags(empty)
   Title: "Add Bookmark"
   AcceptButton: "Add"

9. right click a bookmark and choose properties
   contents: name, location, folder, tags, keyword, description, load in sidebar
   Title: "Properties for "title""
   AcceptButton: "Save Changes"

10. right click a livemark and choose properties
   contents: name, feed location, site location, folder, description
   Title: "Properties for "title""
   AcceptButton: "Save Changes"

11. right click a folder and choose properties
   contents: name, folder, description
   Title: "Properties for "title""
   AcceptButton: "Save Changes"

12. right click a smart bookmark and choose properties
   contents: name, folder, description
   Title: "Properties for "title""
   AcceptButton: "Save Changes"

13. open 2 tabs, from bookmarks menu choose Bookmark all tabs
   contents: name("[Folder Name]"), folder(bookmarks menu)
   Title: "Bookmark All Tabs"
   AcceptButton: "Add Bookmarks"

14. "Bookmark this Link" in context menu of a hyperlink in the content area
   contents: name(filed), folder(bookmarks menu), Tags
   Title: "Add Bookmark"
   AcceptButton: "Add"

15. "Bookmark this Link" in context menu of an already bookmarked hyperlink in the content area
   contents: name, location, folder, tags, keyword, description, load in sidebar
   Title: ...

Read more...

Revision history for this message
In , Mak77 (mak77) wrote :

also, i think we agree all dialogs should be modal since we instant apply, actually only complete dialogs are, while minimal ui is modal only on OS X, so for example if i choose "Bookmark All Tabs" the dialog on Windows is not modal, so the user could close the browser without closing the dialog, losing bookmarks.

Revision history for this message
In , Mak77 (mak77) wrote :

Created an attachment (id=344262)
patch

first take on the new implementation, instant apply inside a dialog.

As requested by Mano i've tried to touch editItemOverlay at a minimum:
- i changed order of the roots to be consistent with the order we have all around in the app (toolbar, menu, unsorted)
- moved icon styling to editBookmarkOverlay.css to be used in all dialogs (and in the menulist).
- overriden method _showHideRows().
- added a string for New Folder to editBookmarkOverlay.dtd

fixes also: bug 433484, Bug 432848

this requires string changes so it would be better landing before the string freeze on 30th. Due to this maybe we could take this as a first step and fix styling and fields later in bug 459958, unless i can get final specifications from Alex in a couple days.

Mano if you agree with the new approach used in this patch but have too many reviews in your queue, you could do a first pass and then i could ask final review to Dietrich (if he has some time slot).

Revision history for this message
In , Mak77 (mak77) wrote :

(From update of attachment 344262)
mh i've fixed another couple things, the override is useless, new version coming

Revision history for this message
In , Mak77 (mak77) wrote :

Created an attachment (id=344265)
patch 1.1

new in this patch:
- removed useless overridde
- don't show folderpicker when editing
- don't ask for tags when adding a keyword

We should practically be at the same visual point as actual version, plus the tags field and the dialog when choosing Bookmark this link in the content.

Revision history for this message
In , Mak77 (mak77) wrote :

trybuilds here:
https://build.mozilla.org/tryserver-builds/2008-10-22_04:<email address hidden>/

Revision history for this message
In , Mak77 (mak77) wrote :

Alex you can test with trybuilds in https://bugzilla.mozilla.org/show_bug.cgi?id=411261#c45, the final result is slightly different from comment 4, more similar to actual behaviour... so i think it would be easier for you trying the build and put comments over dialogs

Revision history for this message
In , Faaborg (faaborg) wrote :

I'll try to get to this soon, if I don't reply within 48 hours can you ping me again.

Revision history for this message
In , Mak77 (mak77) wrote :

Created an attachment (id=344908)
1. history sidebar / bookmark this link [modal, resizeable]

to help Alex i'm posting screenshots of the different dialogs

Revision history for this message
In , Mak77 (mak77) wrote :

Created an attachment (id=344909)
2. context menu / New Bookmark [modal, not resizeable]

Revision history for this message
In , Mak77 (mak77) wrote :

Created an attachment (id=344910)
3. context menu / New Folder [modal, not resizeable]

Revision history for this message
In , Mak77 (mak77) wrote :

Created an attachment (id=344912)
4. livemark subscribe [modal, resizeable]

Revision history for this message
In , Mak77 (mak77) wrote :

Created an attachment (id=344913)
5. add keyword for this search [modal, resizeable]

Revision history for this message
In , Mak77 (mak77) wrote :

Created an attachment (id=344914)
6. Add sidebar panel [modal, resizeable]

Revision history for this message
In , Mak77 (mak77) wrote :

Created an attachment (id=344916)
7. bookmark a rel="sidebar" link [modal, resizeable]

Revision history for this message
In , Mak77 (mak77) wrote :

Created an attachment (id=344917)
8. drop on Bookmarks in the toolbar [modal, resizeable]

Revision history for this message
In , Mak77 (mak77) wrote :

Created an attachment (id=344918)
9. Properties of a bookmark [modal, not resizeable]

Revision history for this message
In , Mak77 (mak77) wrote :

Created an attachment (id=344920)
10. properties of a livemark [modal, not resizeable]

Revision history for this message
In , Mak77 (mak77) wrote :

Created an attachment (id=344921)
11. properties of a folder [modal, not resizeable]

Revision history for this message
In , Mak77 (mak77) wrote :

Created an attachment (id=344922)
12. properties of a smart bookmark [modal, not resizeable]

Revision history for this message
In , Mak77 (mak77) wrote :

Created an attachment (id=344923)
13. bookmark all tabs [modal, resizeable]

Revision history for this message
In , Mak77 (mak77) wrote :

Created an attachment (id=344924)
14. bookmark this link in content [modal, resizeable]

Revision history for this message
In , Mak77 (mak77) wrote :

Created an attachment (id=344925)
13. bookmark this link in content when bookmark exists [modal, not resizeable]

Revision history for this message
In , Mak77 (mak77) wrote :

when this will land it will need some testing from QA to ensure all dialogs are sanely converted to the new method

Revision history for this message
In , Twalker (twalker) wrote :

Yep, I'll review the completeness of litmus coverage here. Update and/or add test cases as need then plus this when it's complete. thanks Marco

Revision history for this message
In , Faaborg (faaborg) wrote :

Created an attachment (id=345034)
Spacing issues

This isn't a huge deal, but a lot of the dialogs have different spacing between the fields. Ideally the spacing would be the same across the board. Also, the progressive disclosure controls should ideally be the same height as the text field so that it visually groups a little more, and just so things look aligned.

Revision history for this message
In , Faaborg (faaborg) wrote :

In terms of wording, I've been giving some thought to the window titles and commit buttons for each dialog.

-I think we should alter the titles to be a little closer to the action the user just took

-In cases where there is pre-populated information, I think the commit button should just be "Save" (not "Save Changes" or "Add"). This is because the dialogs are instant apply, and "Save Changes" implies the changes haven't been saved yet (they have), and "Add" implies it hasn't been added yet (it has).

-In cases where there is no pre-populated information, I think the commit button should be "Add." This is intentionally inaccurate, the bookmark or folder has already been created, but I think it reads a little better and feels more natural.

-In cases where the user is clicking a link to get a dialog box to add a new bookmark, the commit button should be "Add," since the pre-populated information is coming from a third party, and they might not have even expected to see a dialog when they clicked on the link.

Here is all the suggested text in the form:

user action > "window title text" > "commit button text"

1. history sidebar / bookmark this link > "New Bookmark" > "Save"
2. context menu / New Bookmark > "New Bookmark" > "Add"
3. context menu / New Folder > "New Folder" > "Add"
4. livemark subscribe > "Subscribe to Live Bookmark" > "Subscribe"
5. add keyword for this search > "New Bookmark" > "Save"
6. Add sidebar panel > "New Bookmark" > "Add"
7. bookmark a rel="sidebar" link > "New Bookmark" > "Add"
8. drop on Bookmarks in the toolbar > "New Bookmark" > "Save"

9. Properties of a bookmark > "Properties for [name]" > "Save"
10. properties of a livemark > "Properties for [name]" > "Save"
11. properties of a folder > "Properties for [name]" > "Save"
12. properties of a smart bookmark > "Properties for [name]" > "Save"

13. bookmark all tabs > "New Bookmarks" > "Add Bookmarks"
14. bookmark this link in content > "New Bookmark" > "Save"
13. bookmark this link in content when bookmark exists > "Properties for [name]" > "Save"

I think for 6 and 7 we should show the URL (since hte user hasn't seen it yet), and we should show the check box "load this bookmark in the sidebar", with the box checked.

How are 3 and 13 instant apply if they don't have a folder name yet?

Revision history for this message
In , Mak77 (mak77) wrote :

(In reply to comment #24)
> How are 3 and 13 instant apply if they don't have a folder name yet?

I use the default folder name

so also on IRC we ponted out that about:blank for a new bookmark should be hidden if possible.

actually I could fix code and strings, and let further css refinements like spacing for a second pass after the string freeze

Revision history for this message
In , Mak77 (mak77) wrote :

Created an attachment (id=345102)
patch 1.2

This implements all requests from Alex in bug 459958, final work could require some polish on Linux or Mac, i've actually setup a linux build env to test and eventually fix styling, however this polish can be done in bug 459958 before the code freeze, i would prefer taking this before the string freeze instead.

Since Mano's reviews queue is quite long and i don't touch anymore his work in editBookmarkOverlay, i'm moving review request to Dietrich.

Revision history for this message
In , Mak77 (mak77) wrote :

Created an attachment (id=345283)
patch 1.3

fixes a couple styling issues on Linux and Windows

Revision history for this message
In , Luke-iliffe (luke-iliffe) wrote :

The windows zip try server build in comment 45 does not appear to fix bug 432848
as suggested in comment 42. On XP I am seeing the bookmarks menu icon used even when the unsorted bookmarks or bookmarks toolbar option is selected.

Revision history for this message
In , Mak77 (mak77) wrote :

(In reply to comment #50)
> The windows zip try server build in comment 45 does not appear to fix bug
> 432848
> as suggested in comment 42. On XP I am seeing the bookmarks menu icon used even
> when the unsorted bookmarks or bookmarks toolbar option is selected.

thanks for reporting, will look into it, i was seeing this working

Revision history for this message
In , Mak77 (mak77) wrote :

Created an attachment (id=345419)
patch 1.4

fixes icons in menulist (for real this time)

Revision history for this message
In , Dietrich-mozilla (dietrich-mozilla) wrote :
Download full text (3.3 KiB)

(From update of attachment 345419)

> bookmarkLink: function PCH_bookmarkLink(aParent, aURL, aTitle) {
> var linkURI = makeURI(aURL);
> var itemId = PlacesUtils.getMostRecentBookmarkForURI(linkURI);
>- if (itemId == -1) {
>- StarUI.beginBatch();
>- var txn = PlacesUIUtils.ptm.createItem(linkURI, aParent, -1, aTitle);
>- PlacesUIUtils.ptm.doTransaction(txn);
>- itemId = PlacesUtils.getMostRecentBookmarkForURI(linkURI);
>+ if (itemId == -1)
>+ PlacesUIUtils.showMinimalAddBookmarkUI(linkURI, aTitle);
>+ else {
>+ PlacesUIUtils.showItemProperties(itemId,
>+ PlacesUtils.bookmarks.TYPE_BOOKMARK);
> }

can move linkURI into the |if| block

>+ // Load In Sidebar
>+ this._loadInSidebar = PlacesUtils.annotations
>+ .itemHasAnnotation(this._itemId,
>+ LOAD_IN_SIDEBAR_ANNO);
>+ break;

hrm, would be nice to someday centralize all internal anno usage like this in get/set* methods in PlacesUtils.

>+ // When collapsable elements change their collapsed attribute we must

spelling nit: collapsible

> _containsValidURI: function BPP__containsValidURI(aTextboxID) {
> try {
>- var value = this._element(aTextboxID).value;
>+ let value = this._element(aTextboxID).value;
> if (value) {
>- var uri = PlacesUIUtils.createFixedURI(value);
>+ let uri = PlacesUIUtils.createFixedURI(value);
> return true;
> }

is there a practical benefit to this? otherwise it muddies blame unnecessarily.

> _getDescriptionAnnotation:
> function BPP__getDescriptionAnnotation(aDescription) {
>- var anno = { name: DESCRIPTION_ANNO,
>+ let anno = { name: DESCRIPTION_ANNO,

ditto, and all others

>+ //XXX TODO: this should be in a transaction!
> if (this._charSet)
> PlacesUtils.history.setCharsetForURI(this._uri, this._charSet);

file followup?

>+
>+ // Set a selectedIndex attribute to show special icons
>+ this._folderMenuList.setAttribute("selectedIndex",
>+ this._folderMenuList.selectedIndex);

this updates all the special icons for the static items?

>diff --git a/browser/locales/en-US/chrome/browser/places/bookmarkProperties.properties b/browser/locales/en-US/chrome/browser/places/bookmarkProperties.properties
>--- a/browser/locales/en-US/chrome/browser/places/bookmarkProperties.properties
>+++ b/browser/locales/en-US/chrome/browser/places/bookmarkProperties.properties
>@@ -1,13 +1,15 @@ dialogAcceptLabelAddItem=Add
> dialogAcceptLabelAddItem=Add
>+dialogAcceptLabelSaveItem=Save
>+dialogAcceptLabelAddLivemark=Subscribe
> dialogAcceptLabelAddMulti=Add Bookmarks
>-dialogAcceptLabelEdit=Save Changes
>-dialogTitleAddBookmark=Add Bookmark
>-dialogTitleAddLivemark=Add Live Bookmark
>-dialogTitleAddFolder=Add Folder
>-dialogTitleAddMulti=Bookmark All Tabs
>+dialogAcceptLabelEdit=Save
>+dialogTitleAddBookmark=New Bookmark
>+dialogTitleAddLivemark=Subscribe to Live Bookmark
>+dialogTitleAddFolder=New Folder
>+dialogTitleAddMulti=New Bookmarks

Hm, you "subscrib...

Read more...

Revision history for this message
In , Mak77 (mak77) wrote :

(In reply to comment #53)
> >- var uri = PlacesUIUtils.createFixedURI(value);
> >+ let uri = PlacesUIUtils.createFixedURI(value);
> > return true;
> > }
>
> is there a practical benefit to this? otherwise it muddies blame unnecessarily.

i don't want to mixup styles, so i've preferred moving all to one common, could be let or var. So maybe you're right and i'll move all to var to not pollute blame.

> >+ //XXX TODO: this should be in a transaction!
> > if (this._charSet)
> > PlacesUtils.history.setCharsetForURI(this._uri, this._charSet);
>
> file followup?

filed bug 462310

>
> >+
> >+ // Set a selectedIndex attribute to show special icons
> >+ this._folderMenuList.setAttribute("selectedIndex",
> >+ this._folderMenuList.selectedIndex);
>
> this updates all the special icons for the static items?

this allows to recognize different icons in the stylesheet, since we know the position of special items (menu/toolbar/unfiled we can style based on the position. This is actually used to style those 3 items

>
> >diff --git a/browser/locales/en-US/chrome/browser/places/bookmarkProperties.properties b/browser/locales/en-US/chrome/browser/places/bookmarkProperties.properties
> >--- a/browser/locales/en-US/chrome/browser/places/bookmarkProperties.properties
> >+++ b/browser/locales/en-US/chrome/browser/places/bookmarkProperties.properties
> >@@ -1,13 +1,15 @@ dialogAcceptLabelAddItem=Add
> > dialogAcceptLabelAddItem=Add
> >+dialogAcceptLabelSaveItem=Save
> >+dialogAcceptLabelAddLivemark=Subscribe
> > dialogAcceptLabelAddMulti=Add Bookmarks
> >-dialogAcceptLabelEdit=Save Changes
> >-dialogTitleAddBookmark=Add Bookmark
> >-dialogTitleAddLivemark=Add Live Bookmark
> >-dialogTitleAddFolder=Add Folder
> >-dialogTitleAddMulti=Bookmark All Tabs
> >+dialogAcceptLabelEdit=Save
> >+dialogTitleAddBookmark=New Bookmark
> >+dialogTitleAddLivemark=Subscribe to Live Bookmark
> >+dialogTitleAddFolder=New Folder
> >+dialogTitleAddMulti=New Bookmarks
>
> Hm, you "subscribe" to a feed, not to a Live Bookmark. Maybe "New Live
> Bookmark" for consistency w/ the others?
>
> Also, ask Faaborg or Beltzner for ui-r for these string changes.

These string changes have been required by Faaborg himself in https://bugzilla.mozilla.org/show_bug.cgi?id=459958#c24, the idea is to make buttons and titles closer to the actual action, since in the dialog you see the feed name Subscribe does not sound badly, so the user knows he is continuing the subscribe action he has started some second before.

Revision history for this message
In , Mak77 (mak77) wrote :

(In reply to comment #53)
> >+ // Load In Sidebar
> >+ this._loadInSidebar = PlacesUtils.annotations
> >+ .itemHasAnnotation(this._itemId,
> >+ LOAD_IN_SIDEBAR_ANNO);
> >+ break;
>
> hrm, would be nice to someday centralize all internal anno usage like this in
> get/set* methods in PlacesUtils.

do you mean having PlacesUtils.getLoadInSidebar() so all const can be defined in one common place? that could be nice yes

Revision history for this message
In , Luke-iliffe (luke-iliffe) wrote :

When expanding the tag box into a list view to show all tags it is not aligned with the other boxes in the dialog. Will this be covered by Bug 413053 or is it a separate piece of work?

Revision history for this message
In , Mak77 (mak77) wrote :

(In reply to comment #56)
> When expanding the tag box into a list view to show all tags it is not aligned
> with the other boxes in the dialog. Will this be covered by Bug 413053 or is it
> a separate piece of work?

Thanks for following this work, Bug 413053 is for align those views, won't be done here.

Revision history for this message
In , Luke-iliffe (luke-iliffe) wrote :

So to be clear will the alignment of the tree & list views in this bookmark properties dialog be covered in this bug or should a new one be filed?

Revision history for this message
In , Mak77 (mak77) wrote :

(In reply to comment #58)
> So to be clear will the alignment of the tree & list views in this bookmark
> properties dialog be covered in this bug or should a new one be filed?

to be clear, will be done IN Bug 413053, so no need to file a new one.

Revision history for this message
In , Mak77 (mak77) wrote :

(In reply to comment #53)
> (From update of attachment 345419 [details])
>
> > bookmarkLink: function PCH_bookmarkLink(aParent, aURL, aTitle) {
> > var linkURI = makeURI(aURL);
> > var itemId = PlacesUtils.getMostRecentBookmarkForURI(linkURI);
> >- if (itemId == -1) {
> >- StarUI.beginBatch();
> >- var txn = PlacesUIUtils.ptm.createItem(linkURI, aParent, -1, aTitle);
> >- PlacesUIUtils.ptm.doTransaction(txn);
> >- itemId = PlacesUtils.getMostRecentBookmarkForURI(linkURI);
> >+ if (itemId == -1)
> >+ PlacesUIUtils.showMinimalAddBookmarkUI(linkURI, aTitle);
> >+ else {
> >+ PlacesUIUtils.showItemProperties(itemId,
> >+ PlacesUtils.bookmarks.TYPE_BOOKMARK);
> > }
>
> can move linkURI into the |if| block

this confused me too at a first read, but i can't since getMostRecentBookmarkForURI is using it

Revision history for this message
In , Mak77 (mak77) wrote :

Created an attachment (id=345470)
patch 1.5

addressed comments

Revision history for this message
In , Mak77 (mak77) wrote :
Revision history for this message
In , Mak77 (mak77) wrote :

next nightly will contain changes, so feel free to add suggestions here (that are not part of known bug 413053)

Revision history for this message
In , Beltzner (beltzner) wrote :

(In reply to comment #24)
> -In cases where there is no pre-populated information, I think the commit
> button should be "Add." This is intentionally inaccurate, the bookmark or
> folder has already been created, but I think it reads a little better and
> feels more natural.

This makes sense assuming that there is an associated "Cancel" button which destroys the newly created resource. That indeed seems to be the case as far as I can tell.

Those strings all look much better, yes.

> How are 3 and 13 instant apply if they don't have a folder name yet?

Items I create using 3 don't seem to show up until I click "Add", fwiw. Or is that part of the change you're making? It's important that in those cases "Cancel" means "don't actually create the thing" as opposed to "don't rename the thing from the default name to what I just said".

Revision history for this message
In , Francesco-lodolo (francesco-lodolo) wrote :

* editBookmarkOverlay.newFolderButton.accesskey and editBookmarkOverlay.name.accesskey share the same value: is it ok (different scope)?

* "Subscribe to Live Bookmark": AFAIK in Firefox you can subscribe to a feed using Live Bookmarks (see for example subscribeFeedUsing="Subscribe to this feed using" in subscribe.properties), the action of subscribing to a Live Bookmark seems a bit "strained"

Revision history for this message
In , Mak77 (mak77) wrote :

(In reply to comment #63)
> * editBookmarkOverlay.newFolderButton.accesskey and
> editBookmarkOverlay.name.accesskey share the same value: is it ok (different
> scope)?

thanks for this, has to be fixed, Faaborg, what do you think would be better to assign here?

> * "Subscribe to Live Bookmark": AFAIK in Firefox you can subscribe to a feed
> using Live Bookmarks (see for example subscribeFeedUsing="Subscribe to this
> feed using" in subscribe.properties), the action of subscribing to a Live
> Bookmark seems a bit "strained"

This was expressely asked to both Faaborg and Beltzner, they think it's ok, hwv it's their choice to change it.

Revision history for this message
In , Mak77 (mak77) wrote :

(In reply to comment #27)
> > How are 3 and 13 instant apply if they don't have a folder name yet?
>
> Items I create using 3 don't seem to show up until I click "Add", fwiw. Or is
> that part of the change you're making? It's important that in those cases
> "Cancel" means "don't actually create the thing" as opposed to "don't rename
> the thing from the default name to what I just said".

It should appear as New Folder, and if you click Cancel it should disappear. It's working like that for me.

Revision history for this message
In , Beltzner (beltzner) wrote :

(From update of attachment 345470)
ui-r+ with the following nit/change as per localizer comments:

+dialogAcceptLabelAddLivemark=Subscribe
+dialogTitleAddLivemark=Subscribe with Live Bookmark

Quite right that you don't subscribe to the Live Bookmark; you subscribe to the feed WITH a Live Bookmark. Alex's design was right, though, to carry the verb that we use elsewhere in the UI forward into this dialog, though.

Revision history for this message
In , Mak77 (mak77) wrote :

Created an attachment (id=345631)
followup

Followup to fix locale as required in previous comment by Beltzner, accesskey for New Folder is o per IRC.

Revision history for this message
In , Mak77 (mak77) wrote :
Revision history for this message
In , Mak77 (mak77) wrote :

*** Bug 422193 has been marked as a duplicate of this bug. ***

Revision history for this message
In , Twalker (twalker) wrote :

verified in nightly build of 2008103102 on Mac

also updated litmus test case: https://litmus.mozilla.org/show_test.cgi?id=5952

Revision history for this message
In , L10n-mozilla (l10n-mozilla) wrote :

Doing the string change without a key change is borderline. You may read the original string as obviously broken or not, and either localizers would catch that magically or not.

Mind doing a quick post to mozilla.dev.l10n to make sure that localizers see that there is an update to an existing string,

-dialogTitleAddLivemark=Subscribe to Live Bookmark
+dialogTitleAddLivemark=Subscribe with Live Bookmark

in browser/places/bookmarkProperties.properties.

CCing our l10n watcher to get some bugmail noise out, too.

Revision history for this message
In , L10n-mozilla (l10n-mozilla) wrote :

... and earlier changes to that file, too.

Dietrich, patches like that should get an r-.

Revision history for this message
In , Mak77 (mak77) wrote :

i posted in mozilla.dev.l10n, really sorry for that Axel, was my fault :(

Revision history for this message
In , Mak77 (mak77) wrote :

*** Bug 465050 has been marked as a duplicate of this bug. ***

Revision history for this message
In , Mak77 (mak77) wrote :

*** Bug 419713 has been marked as a duplicate of this bug. ***

Revision history for this message
In , Dmcritchie (dmcritchie) wrote :

Note that Bug 419713 which was marked as a duplicate has some workarounds in "Open Book" extension, and customizable in User style 9029. Keywords should should also show from Organize Bookmarks (bottom), the location bar star, and of course Ctrl+D. Mentioned particularly because in Organize Bookmarks Description etc will take up space from main list, so would want specific control there of what appears there before using the More button.

Revision history for this message
In , Hskupin (hskupin) wrote :

Edward, I believe this is a dupe of bug 392143?

Revision history for this message
In , Mak77 (mak77) wrote :

*** Bug 470915 has been marked as a duplicate of this bug. ***

Revision history for this message
In , Mak77 (mak77) wrote :

Alex, what is left to do here from a UI perspective?

Revision history for this message
In , Shawn Wilsher (sdwilsh) wrote :

*** This bug has been marked as a duplicate of bug 405795 ***

Revision history for this message
In , Aleksej (aleksejrs) wrote :

This bug is about the keyword, and that bug includes the URL and proposes a different solution.

Revision history for this message
In , Jh-junetz (jh-junetz) wrote :

(In reply to comment #7)
> Edward, I believe this is a dupe of bug 392143?

That's a Firefox bug, this is not.

Revision history for this message
In , Jh-junetz (jh-junetz) wrote :

*** Bug 498086 has been marked as a duplicate of this bug. ***

Revision history for this message
In , Adw-mozilla (adw-mozilla) wrote :

*** Bug 395109 has been marked as a duplicate of this bug. ***

Revision history for this message
In , Mak77 (mak77) wrote :

Unless Alex has something against this (in such a case feel free to reopen and elaborate on that), i'm going to mark this as WFM based on changes in bug 411261 and followers, further changes should have their own bug.

Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9.1pre) Gecko/20090619 Shiretoko/3.5pre

Revision history for this message
In , Bugzilla2007 (bugzilla2007) wrote :

Aleksej and reporter, I agree very much with the goal of this bug, but I think it should be resolved duplicate of 405795 as proposed by Shawn in comment #7, for the following reasons:

1) This bug covers a very small subset (make *Keywords* accesible via "Add/Edit this bookmark" / Bookmark Properties dialogues) of bug 405795 (make *all properties* accessible for the dialogues).

2) If your preferred solution is to have the Keywords property visible /at all times/ (without ever clicking more-button to expand the dialogue): this won't happen. Bug 405795 comment #15 leaves no doubt that the minimal ("streamlined" / "crippled") design of the dialogue is intended and only light-weight ("non-obtrusive") additions will be accepted. As much as I am in favour of making the dialogue expandable to edit all properties, I think adding only "Keywords" property permanently to the reduced(!) dialogue would be very confusing: Average user won't know the difference between tags and keywords.

3) With 2) in mind, I think there are only 2 possible solutions for this bug:

a) make reduced "Edit this bookmark" dialogue expandable (more-button) to edit all the properties of a bookmark /inside/ the dialogue. IMO, this is the most-needed and most intuitive option.

b) add a button that links to some other way of editing all properties /outside/ the reduced dialogue (all properties window, "Open containing folder in Library" etc.). IMO, this alone(!) makes it unnecessarily complex to edit the props.

c) combine a) and b): expandable properties dialogue AND "Open containing folder in Library" button (cf. bug 469441). IMO, this is the best solution with added value.

All these possible solutions are covered and discussed in bug 405795. I have proposed a sample UI of the expandable dialogue in bug 405975 attachment 392075, which solves the problem of this bug by combining the best of a) and b).
In other words: we should focus our energy on finding a good solution for bug 405795 that makes everyone happy.

Aleksej and reporter, any objections against resolving this bug as a duplicate?

Revision history for this message
In , Bugzilla2007 (bugzilla2007) wrote :

Can we please remove wontfix? nomination from this bug?
It's an insult in the face of 10 votes, several duplicates, more votes at bug 405795, and especially the outstanding popularity of Firefox's unique "smart keyword" feature, as can be seen from google search results like the following:
http://www.google.com/search?q=Firefox%20bookmark%20keywords

Revision history for this message
In , Shawn Wilsher (sdwilsh) wrote :

(In reply to comment #10)
> Can we please remove wontfix? nomination from this bug?
No, unless a module owner does so. It's nominated for wontfix by a module peer, and the module owner will take a look at it at some point.

Revision history for this message
In , Bugzilla2007 (bugzilla2007) wrote :

It's good to know that you have your hierarchies all sorted. We also get a sense where the user might be in that hierarchy: somewhere far down at low interest level. To assist with taking an informed look at it "at some point", here's some references both future and past all asking for the same thing: Can we please have some easy way of adding a keyword while bookmarking pages?

http://forums.mozillazine.org/viewtopic.php?f=8&t=1431655
https://bugs.launchpad.net/firefox/+bug/237679
http://forums.mozillazine.org/viewtopic.php?f=38&t=552496&start=0
http://forums.mozillazine.org/viewtopic.php?f=23&t=687835&start=0
http://forums.mozillazine.org/viewtopic.php?t=650888
http://forums.mozillazine.org/viewtopic.php?f=7&t=179600
http://forums.mozillazine.org/viewtopic.php?p=350092
http://forums.mozillazine.org/viewtopic.php?f=38&t=482646&p=2578288

Revision history for this message
In , Mak77 (mak77) wrote :

This usage of keywords looks like misuse as tags, keywords adding natural UI is: right click a search field and choose "add a keyword for this search". Otherwise looks like you want a tag on your bookmark.
We need clear use-cases that are not "type g and press enter to go to Google" obviously.

And the awesomebar can find bookmarks quite efficiently typing partial content, thus removing the need for a short "nickname" for the bookmark that we had in FX2.

Users are on top of the hierachy, but even if Firefox is built to accomplish satisfaction in most users, it just can't satisfy every user. That's why we have a user experience team, module owners and so on, to be able to take decisions that we think are good for most users, but maybe they won't be for all of them (with hundreds millions users you can understand how hard that can be). And that's also why extensions do exist, and i'm pretty sure an extension that does that already exists as of today.

that said, i won't spam anymore here, all the needed requests are filed.

Revision history for this message
In , Bugzilla2007 (bugzilla2007) wrote :
Download full text (3.8 KiB)

(In reply to comment #13)
Thanks for that comment, it was helpful to understand your personal perception of keywords and thus your problem with this bug. Marco, are you using keywords yourself? As a location bar keyboard shortcut for opening URLs with and without query parameters? Looking at comment #13, I guess not...

> This usage of keywords looks like misuse as tags, keywords adding natural UI
> is: right click a search field and choose "add a keyword for this search".
> Otherwise looks like you want a tag on your bookmark...
> And the awesomebar can find bookmarks quite efficiently typing partial
> content, thus removing the need for a short "nickname" for the bookmark that
> we had in FX2.

Absolutely NOT. You _cannot_ use a tag instead of a keyword as a keyboard shortcut to _reliably_ open a static URL with as few steps as possible. The functionality is very different, and in most cases, it simply won't work (since location bar autocomplete is based on frecency more than on keyword matches, and they're competing with your history matches). Even defining a _unique_ tag will _not_ ensure autocomplete match from location bar, so tags are pretty useless as a shortcut for specific urls (I can provide STR for a test case, if needed). Whereas the _keyword_ shortcut works 100% of the time, and guaranteed hassle-free: Ctrl+L, type keyword, press Enter - you're there. Works like a charme, without thinking, parsing, cursoring and everything, straight from your muscle memory. Always.

So keywords provide valuable functionality for highly efficient browsing; therefore, adding keywords to a bookmark should be easy. It's way too hard now (especially when creating a new bookmark; quote below from http://forums.mozillazine.org/viewtopic.php?f=8&t=1431655):

-------------------------------------------------------------
Actual Behaviour (missing on comment #0):

> Right now, to assign a keyword, we have to do this:
>
> . select bookmark this page
> . ctrl+shift+B
> . search for the bookmark
> . select "more"
> . enter the keyword
> . close
>
> #-o [-( The insanity of it all!!! 8-[
-------------------------------------------------------------
Yes, it's insane. Therefore this bug. Access to all properties of a bookmark from it's most prominent representation (the star) should NOT need extensions.

A simple fix that can make virtually everyone happy has been proposed in bug 405795:
-> Add some sort of tiny [v] more button to expand the crippled "Edit this bookmark" dialogue (cf. mockup screenshots, attachment 391888, attachment 392075, attachment 392887)

The fact that we are deliberately hiding valuable functionality (keywords) to an extent that the average user will never find it, is also detrimental to Firefox adoption (cf. comment #0):
> ...but why, oh why are bookmark keywords so shunned! They're an awesome
> feature, but hardly anybody knows about them because you don't even
> see them unless you venture into bookmark manager.

From a Mozilla inside perspective, the article "How Cool are Custom Keywords" by Mozilla's Asa Dotzler looks pretty much supportive of this view (http://www.mozilla.org/docs/end-user/keywords.html). I know Asa from the Mozilla ...

Read more...

Revision history for this message
In , Bugzilla2007 (bugzilla2007) wrote :

(In reply to comment #13)
> We need clear use-cases that are not "type g and press enter to go to Google"
> obviously.

OK, maybe this usage quoted from "How Cool are Custom Keywords?" (by Mozilla's Asa Dotzler) is good enough? (http://www.mozilla.org/docs/end-user/keywords.html, my emphasis):

> Mozilla Custom Keywords ROCK! Not just for making *shorthand for bookmarks*
> but also for searches and queries. Simple keywords allow you to type a short
> string in the Location Bar and load its corresponding Bookmark URL.
>
> *An example is my bookmark for http://www.mozilla.org to which I've added the
> keyword "m.o".* So, with that set, I can type "m.o" in the Location Bar and
> it loads http://www.mozilla.org. Using keywords combined with autocomplete in
> Mozilla and I seldom type more than about three or four characters for all of
> the sites I regularly visit.

As requested by comment #13, here's some more "clear use-cases that are not 'type g and press enter to go to Google'" (some of my keyword shortcuts for loading _static_ URLs):

mo -> www.mozilla.org (Sites of personal interest; all with similar names and thus usually not directly addressable via location bar matches)
mz -> www.mozillazine.org
mdc -> https://developer.mozilla.org/En (*)
xul -> https://developer.mozilla.org/En/XUL (*)
bzo -> www.bugzilla.org (important portals)
wpo -> www.wikipedia.org (reference works start pages)
sh -> www.selfhtml.org
shf -> http://forum.de.selfhtml.org (forum entry pages of all sorts)
he -> http://htmledit.squarefree.com/ (Real time HTML Editor)
abc -> www.abc-school.de (my workplace; this is not the real link)
ob -> www.my-bank.de/onlinebanking (my bank account; not the real link)
ucc -> www.xe.com/ucc/ (Universal currency converter)
yt -> www.youtube.com (sites where the start page provides appetizers)
zw -> www.sokwanele.com/thisiszimbabwe (news sites of personal interest)

All of these I am using on a daily basis for the fastest possible access to my favourite sites (_static_ URLs). In addition, I have a lot of smart keywords for "query URLs". With millions of users, I'm sure a _lot_ of people will use this in a similar way. HTH.

(*) "mdc" and "xul" as shortcut keyword for Mozilla Devolper Center / XUL Project are two more good examples why tagging just won't do the trick: after typing "mdc" / "xul" in location bar, you'd get countless matching pages from your browsing history on MDC / XUL. On the other hand, hit "Enter" after your favorite shortcut keyword and it'll get you straight to the respective pages. No fuss, 100% of the time.

> keywords adding natural UI is: right click a search field and choose "add a
> keyword for this search".

Unfortunately, we can't set keywords for _static_ urls using this method.
In addition, even for query URLs, the context menu method won't work for certain cases (e.g. http://maps.google.com/) and will create untidy URLs with unknown and unnecessary parameters set. All of these problems would be easily cured by making all bookmark properties accessible from the respective bookmark dialogues (Bug 405795).

Revision history for this message
In , Mak77 (mak77) wrote :

We will provide an updated bookmarking UI soon, that *could* satisfy your needs, as it could not. Firefox is designed to satisfy most users, not always any user, unfortunatly. You are free to partecipate to the design discussions with our UX team. We will soon post mockups to get feedback, as we are used to do, it's wrong to think we don't listen, we just have to make choices.

I see your passion, and it's fine, but please be polite toward us.

This bugs as "extend current star panel with additional fields", even if hidden, is still wontfix imo, but new design could probably have space to reduce the user road toward keywords.

Revision history for this message
In , Bugzilla2007 (bugzilla2007) wrote :
Download full text (3.2 KiB)

(In reply to comment #14)
> (In reply to comment #13)
> You _cannot_ use a tag instead of a keyword as a keyboard
> shortcut to _reliably_ open a static URL with as few steps as possible. ...
> In most cases, it simply won't work (since location bar autocomplete is based
> on frecency more than on keyword matches,

typo fix: Using tag instead of keyword often won't work because location bar autocomplete finds and orders matches based on frecency more than based on _*tag*_ matches, and so tag matches are competing with your history matches, which are likely to be listed near the top.

So while a bookmark with matching tag might not even show on the autocomplete list at all, a bookmark with a matching keyword will always show at the very top of autocomplete list (but I won't even need the list as I just use the keyword).

(In reply to comment #16)
> We will provide an updated bookmarking UI soon,

Let's hope that this is good news.

> Firefox is designed to satisfy most users, not always any user, unfortunatly.
> ...we just have to make choices.

I fully understand and accept this. My point is that in this particular case, there are two proposed solutions in bug 405795 that could satisfy everyone without losing anything (don't change the reduced dialogue so as to keep it simple, but add a tiny toggle to make it expandable as we do in bookmark manager properties pane).

> We will soon post mockups to get feedback, as we are used to
> do, it's wrong to think we don't listen

Thank you for listening, and I'm sure you do. I think it's when easy solutions with a lot of benefit but no or virtually no negative impact get rejected without good arguments (or even wrong arguments), that we might feel not listened to. I know it's tricky because "good arguments" are hard to define, and tend to vary from different points of view.

Where will the mockups be posted? Can you leave a note on this bug when they are?

> This bugs as "extend current star panel with additional fields", even if
> hidden, is still wontfix imo, but new design could probably have space to
> reduce the user road toward keywords.

I accept that we want to keep the reduced star panel as a default, but unfortunately I'm perfectly failing to see what's wrong with making it expandable (which really seems the shortest and most intuitive road)...
As a way out, maybe we can try along Alex' proposal of bug 405795, comment #20 (completely unobtrusive "expand to window" icon, tear-off-approach), explained in bug 405795, comment #25 and welcomed by Marco in bug 405795, comment #24. :)

Marco, you think we could duplicate this against bug 405795 (as proposed by Shawn in comment #7, and explained in comment #9 with no reply or objection from reporter or Aleksej, who wanted to keep this separate in comment #8)?
Inaccessibility of keywords from "Edit this bookmark" dialogue seems to be a subset of bug 405795 in that Description, URL (cf. bug 518423), and bookmark's folder are also inaccessible, and if anything, we'd probably want a unified approach that makes _all_ properties available somehow...

Finally (@FF devs and contributors), thanks for all the good work you do, we've seen some great improvem...

Read more...

Revision history for this message
In , Mak77 (mak77) wrote :

(In reply to comment #17)
> Where will the mockups be posted? Can you leave a note on this bug when they
> are?

bug 524071 is aimed to add an UI that will provide all fields at a click distance, the current idea is a right sidebar. Please avoid long comments, try to be concide, Alex needs a clean bug, so any suggestion should be expressed in a few words and/or point to a newgroups discussion in moz.dev.apps.firefox.

> I accept that we want to keep the reduced star panel as a default, but
> unfortunately I'm perfectly failing to see what's wrong with making it
> expandable

not all the users are good with expandable UIs, especially basic users tend to forget they can contract panels. moreover that panel was intended to allow really quick bookmarking, and it already has too many controls in it, it should be small and sleak, while already now it appears bloated.

> Marco, you think we could duplicate this against bug 405795 (as proposed by
> Shawn in comment #7, and explained in comment #9 with no reply or objection
> from reporter or Aleksej, who wanted to keep this separate in comment #8)?

Imho that's the right choice, as you want keywords, some other user could want description, and so on, so there is not much difference to bug 405795

Revision history for this message
In , Bugzilla2007 (bugzilla2007) wrote :

Interestingly, Firefox 3.7 / 4.0 are actually heading for a powerful _EXPANSION of keywords magic_ by introducing TASKFOX*, which will see a _massive_ return of a familiar use pattern... :

(from: https://wiki.mozilla.org/Taskfox/Use_Cases)
* Select part of the page you want to send
* Type into address bar: "email this to blair" ["map this" | "weather" | etc.]
* Hit enter

:)))

(In reply to comment #17)
Thanks for pointing out bug 524071, up since 2009-10-23, mentioning that one a bit earlier might have avoided a lot of discussion around here...

*: https://wiki.mozilla.org/Firefox/Projects/3.7_and_4.0_Theme_and_UI_Revamp/Direction_and_Feedback#Merging_the_LocationBar_and_SearchBar

Revision history for this message
In , Bmcbride (bmcbride) wrote :

(In reply to comment #19)
> Interestingly, Firefox 3.7 / 4.0 are actually heading for a powerful _EXPANSION
> of keywords magic_ by introducing TASKFOX*, which will see a _massive_ return
> of a familiar use pattern... :

While one of the ideas I explored in Taskfox was to improve support for the existing keyword functionality, I don't think its relevant to this bug.

Revision history for this message
In , Gervase Markham (gerv-mozilla) wrote :

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

Revision history for this message
In , Brandon (tigernerve) wrote :

I see that no one is working on this bug. I would be willing to volunteer my time to help fix this, yet I don't know much programming. Does anyone know how I can access the code and what computer language this is written in?

<email address hidden>

Revision history for this message
In , Mackenzie Morgan (maco.m) wrote :

Firefox is written in C++ on the backend and XUL (easy if you know HTML) and Javascript on the frontend. Source code info: https://developer.mozilla.org/En/Developer_Guide/Source_Code

Changed in firefox:
importance: Unknown → Medium
Revision history for this message
In , Mak77 (mak77) wrote :

We don't plan to add keywords to the default UI, whether we'll provide an alternative "full-edit" UI is bug 405795, there is no other plan to do partial changes.

*** This bug has been marked as a duplicate of bug 405795 ***

Changed in firefox:
status: Confirmed → Invalid
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.