99860 - File Bookmark dialog needs cleanup

Status: VERIFIED FIXED

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

Description

[Matthew T (active 1999-2002)]

Build: 2001091405

To reproduce:
*   Choose `Bookmarks' > `File Bookmark...'.

Problems with the dialog:
1.  It's non-modal, but should be modal.
2.  It's too small by default.
3.  It's titled `Add Bookmark', instead of `File Bookmark'.
4.  The gaps between the `Name:' and `Location:' labels and their respective
    fields are not consistent.
5.  The `Location:' label should be `Address:'.
6.  The `Create in:' label should appear above the tree, not to the left of it.
7.  The left edge of the folder tree isn't aligned with anything.
8.  The right edges of the controls are crooked.

What you should see:
+--------------------------------------------------+
|::::::::::::::::: File Bookmark ::::::::::::::::::|
+--------------------------------------------------+
|    Name: [Enter Bug                            ] |
| Address: [http://bugzilla.mozilla.org/enter_bug] |
| Keyword: [                                     ] |
|                                                  |
| Destination:                                     |
| +--------------------------------------------+-+ |
| ||> [] Personal Toolbar Folder               |A| |
| ||> [] Mozilla interface complaints          |:| |
| ||> [] Stuff to look at                      |:| |
| ||> [] Daily/Weekly                          |:| |
| |                                            |:| |
| |                                            |:| |
| |                                            |:| |
| |                                            |:| |
| |                                            |:| |
| |                                            |:| |
| +--------------------------------------------+-+ |
|                ( Use Default ) ( New Folder... ) |
|                                                  |
|                            ( Cancel ) ((  OK  )) |
+--------------------------------------------------+

The `Use Default' button can be removed if bug 36339 is fixed first.

Comment 1

[timeless]

1. it should not be modal. what if i want to copy text from the page?
6. is inconsistent w/ [What you should see:]

Comment 2

[Matthew T (active 1999-2002)]

> 1. it should not be modal. what if i want to copy text from the page?

Then you need to copy it first. The same applies to the Save filepicker, and
that's modal too.

> 6. is inconsistent w/ [What you should see:]

Well, it can stay as `Create in:' for now if the implementor so desires; but it
will need to be `Destination:' eventually, once the dialog lets you select not
only the destination folder but also exactly which two items the bookmark should
be created between.

Comment 3

[Gervase Markham [:gerv]]

Attachment: New bookmarks window

Comment 4

[Gervase Markham [:gerv]]

Something like that? :-) You can't have the keywords field because the Bookmark
service's Add Bookmark function doesn't have a parameter for one. There may be a
way to do it, but I'm not going to let that hold this up. Unless someone CCed on
this bug wants to leap in and tell me...

I have a patch if that looks good to you.

Gerv

Comment 5

[Håkan Waara]

I have a patch.

Comment 6

[Håkan Waara]

Aw, dammit, duplicated work!

I also have a patch (just mid-aired with you) that does a few more things.
Cleanup, fixing some more wordings, fixing some spacing issues...

Maybe we could merge our work?

Comment 7

[Gervase Markham [:gerv]]

Then why on earth didn't you say so? I thought it wasn't necessary to mention
that I was writing one half an hour before I knew I'd have to mention that I'd
finished it. When did you write yours?

Gerv

Comment 8

[Gervase Markham [:gerv]]

Attachment: Gerv's patch v.1 - produces attached screenshot

Comment 9

[Håkan Waara]

I can say the same to you: why the hell didn't you mention it. I wrote mine just
now, in about one hour as well.

Comment 10

[Gervase Markham [:gerv]]

How unlikely is that? :-) 

Obviously I have nothing better to do on a Sunday afternoon, and you have
nothing better to do on a Sunday evening. <sigh>.

Would you consider accepting my apologies, and reviewing my patch? :-)

Gerv

Comment 11

[Håkan Waara]

Attachment: Hwaara's first draft patch

Comment 12

[Gervase Markham [:gerv]]

Hakan: did you read the comments at the top of addBookmark.js? This is a
multi-purpose dialog and, if I'm not mistaken, I think you've killed things
vital to some of its other purposes.

Gerv

Comment 13

[Håkan Waara]

First off, sorry for not notifying people in this bug before I started working
on it. mpt CC'ed me so I thought it was kind of implied I should fix it. ;)

Anyway, my patch contains some good stuff yours doesn't, yours has some mine
doesn't - I think we should merge them.

Note that my patch doesn't contain the non-modal -> modal stuff though.

If you don't want to merge, I can do that tomorrow. 

What do you think?

Comment 14

[Håkan Waara]

I removed a separator that is no longer needed with my changes. Otherwise, the
changes will work fine in both "Add Bookmark" from BM manager and File Bookmark
from browser.

However, your patch changes the title to something specific ("File Bookmark")
which  doesn't work for the Add Bookmark dialog.

Comment 15

[Gervase Markham [:gerv]]

> mpt CC'ed me so I thought it was kind of implied I should fix it. ;)

My thoughts exactly :-)

If you'd like to merge them, that would be great. But do read that comment in
addBookmark.js - there are four different ways to invoke this dialog. 

I also don't have the modal/non-modal stuff. I'm not sure how to do that. I
assume it's in the call somewhere.

Gerv

Comment 16

[Jason Kersey]

/xpfe/components/bookmarks/resources/bookmarksOverlay.js#848
/xpfe/components/bookmarks/resources/pref-bookmarks.xul#68

Those are the only two places i found it not to be modal.  There are two others
in bookmarksOverlay.js where they are modal.  You may want to ask Ben why some
are not, he may have good reason.

Comment 17

[Matthew T (active 1999-2002)]

To my untrained eye, those two non-modal cases appear to be from the days where 
we had a Bookmarks prefs panel: the first is from when we used a pref to 
determine whether to show the dialog when adding a bookmark, and the second 
uses the dialog to choose the default folder for new bookmarks (which is just 
broken -- it should be a `Set as Default Folder' menu item in the Bookmarks 
window itself). I suspect that neither of those code paths are used any more.

Ideally, the dialog should be titled `File Bookmark' or `New Bookmark' 
depending on whence it was called.

Note that in attachment 49526 [details] [diff] [review], the `Use Default' and `New Folder...' buttons 
are in the wrong order. (Having `New Folder...' rightmost ensures consistent 
position both with other lists of items which have a `New...' button but no 
`Use Default' button, and with the post-bug-36339 version of this same dialog.)

Comment 18

[Gervase Markham [:gerv]]

Hwaara's taking up the torch here. If it's decided my patch is wanted (for
example, if he doesn't have time), please assign this back to me.

Gerv

Comment 19

[Håkan Waara]

Gerv is right.  Sorry for delaying this fix, my time is up for this one.

Comment 20

[Gervase Markham [:gerv]]

This isn't going to make it this time round.

Gerv

Comment 21

[Gervase Markham [:gerv]]

Attachment: Patch v.2 - refreshed

Comment 22

[Gervase Markham [:gerv]]

Attachment: Patch v.3 - Location instead of Address

Comment 23

[Gervase Markham [:gerv]]

I've refreshed the patch. It still works, and it's still an improvement. CCing
hyatt in case he has UI comments (although this is more cleanup than radical
change.)

hwaara: do you have time to review this?

Gerv

Comment 24

[Håkan Waara]

Can you post a screenshot and -uw patch?

Comment 25

[Gervase Markham [:gerv]]

There's a screenshot already attached; the only change is the word Address is
now back as Location. 

A diff -uw would not be significantly smaller than the current diff -u. There's
very little whitespace-only change.

Gerv

Comment 26

[Håkan Waara]

Comment on attachment 55367 [details] [diff] [review]
Patch v.3 - Location instead of Address

Looks good.  r=hwaara

Comment 27

[Blake Ross]

Please wait until I land bookmarksliner.

Comment 28

[Gervase Markham [:gerv]]

Blake: if you say so; when do you expect that to be?

Also, does this mean the widget in this dialog will change, or just the one in
the Bookmarks window?

Gerv

Comment 29

[Gervase Markham [:gerv]]

blake: do I need to retarget this bug? Which bug tracks bookmarksliner? Can you
set up a dependency, please?

Gerv

Comment 30

[burtonator]

Should the new dialog box also include the "keyword" and "comments" input you
are provided when you use the "Manage Bookmarks" application and select
"Properties"?  I think these two dialog boxes should be similar.

Comment 31

[Gervase Markham [:gerv]]

Pushing out; this isn't worth taking on the branch. There's no rush.

But Blake - can you please set up a dependency on the bookmarksliner bug? Thanks :-)

Gerv

Comment 32

[Gervase Markham [:gerv]]

That would involve more serious changes, because those things are not in the
standard "file bookmark" API call. You'd have to file the bookmark, get a hold
of it, and add these properties.

This is a low-hanging fruit bug. Minimum changes for maximum gain.

Gerv

Comment 33

[R.K.Aa.]

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

Comment 34

[Gervase Markham [:gerv]]

Blake has landed bookmarksliner; but I don't have a Linux box connected to
broadband so fixing up this patch may take some time.

Pushing out to 0.9.8; it may get in under the wire.

Gerv

Comment 35

[A. Craig West]

As of Build 2002042603 on Win2K with Large fonts I am seeing the File Bookmarks
dialog coming up too small. At least it is resizable...

Comment 36

[Pierre Chanial]

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

Comment 37

[Pierre Chanial]

in bug 139591, the reporter requests for a persistent size across sessions

Comment 38

[André Dahlqvist]

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

Comment 39

[Boris Zbarsky [:bzbarsky]]

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

Comment 40

[Eric Vaandering (no email)]

comment 38 points out that the number of entries in the folders list shrinks
from 7 to 2 with the new "File as Group" option, making the dialog hard to use
without a manual resize.

Comment 41

[O'Shaughnessy Evans]

I'd like to confirm that I'm seeing one of the problems described in this bug
report w/the latest nightly build (2002-5-16) for Solaris 2.6 and it really
makes things awkward.  Every time I go to Bookmarks -> File Bookmark..., the
"Create in" bookmark tree is only 2 lines high.  If I resize the window I get a
bigger tree, but by default this makes for a very awkward way to file a bookmark.

By the way, it looks like this is a pretty old bug.  Could someone explain why
one of the patches attached here hasn't been incorporated yet?  I'm not a real
experienced mozilla tester, so please accept my apologies if this is a naive
question.  (Or is this where the "target milestone" field comes in?)

Comment 42

[Pete Boyd]

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

Comment 43

[R.K.Aa.]

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

Comment 44

[Gervase Markham [:gerv]]

> By the way, it looks like this is a pretty old bug.  Could someone explain why
> one of the patches attached here hasn't been incorporated yet?

Hwaara and my implementations clashed; he backed off, and then I ran out of
time. Someone else, feel free to fix this bug :-) I'm not going to be able to in
the near future.

Gerv

Comment 45

[Kalle Valo]

Attachment: Kalle's patch v0.1 - based on Gerv's patch v.3

This patch is based on Gerv's patch v.3, I just updated it a bit. I
moved dialog's height property to folderbox because otherwise
File/New/Bookmark dialog would be too big and made it also smaller.

I have tested this patch with recent trunk on Linux.

N.B. This is my first patch to the Mozilla project ever so handle with
caution.

Comment 46

[Håkan Waara]

Kalle, please attach a screenshot of the result of your patch, so we can compare
it to this bug's spec (see comment 0).

Comment 47

[Kalle Valo]

Attachment: File Bookmark dialog of Kalle's v0.1 patch

Comment 48

[Kalle Valo]

Attachment: New Bookmark dialog of Kalle's v0.1 patch

This is taken from Bookmark manager's File/New/Bookmark menu. It uses same
addBookmark.xul as File Bookmark in main window.

Comment 49

[Håkan Waara]

Comment on attachment 84736 [details] [diff] [review]
Kalle's patch v0.1 - based on Gerv's patch v.3

A few comments:

* "Create in" should be "Destination" per comment 0.
* Switch the ordering of the buttons, per comment 17.
* Does the dialog have a reasonable default size now? Sometimes when I bring it
up in RC2, it's too small...

In general it looks great! Just needs a little bit more work-

Comment 50

[Peter Lairo]

The screenshot (v 0.1 patch) seems to be missing the *File as group* checkbox. :(
--> http://bugzilla.mozilla.org/attachment.cgi?id=84773&action=view

PS. I don't see the point of the "New Bookmark" screenshot. It looks just like
the window in the current builds.
--> http://bugzilla.mozilla.org/attachment.cgi?id=84774&action=view

PPS. This patch doesn't seenm to allow _setting of_, or _adding to_ the *root
folder* ("New Bookmark Folder"). Probably another bug (#?), though.

Comment 51

[Peter Lairo]

This bug is blocking bug 145941 ([Meta] All resizable/movable windows should
remember user selected size/position). Will this bug fix those issues, or should
I file a separate bug for remembering size & position?

Comment 52

[Kalle Valo]

from comment 49:

> "Create in" should be "Destination" per comment 0.

I will change that.

> Switch the ordering of the buttons, per comment 17.

Ok, I will do it.

> Does the dialog have a reasonable default size now? Sometimes when I bring it
> up in RC2, it's too small...

I have seen this too, both in 1.0 branch and trunk. In this patch I set the
height for folderbox vbox to 20em and it seems to resolve the problem. Is there
a better way than this? I'm a shamed to admit but I don't anything about XUL.

from comment 50:

> The screenshot (v 0.1 patch) seems to be missing the *File as group* checkbox.

I didn't have multiple pages open while I was taking the shot and that's why
file as group checkbox was hidden. But 'file as group' box works, I tested it.

> I don't see the point of the "New Bookmark" screenshot. It looks just like
> the window in the current builds.

New Bookmark menu command in Bookmark Manager uses same addBookmark.xul. I
wanted to show that my patch doesn't break that.

> This patch doesn't seenm to allow _setting of_, or _adding to_ the *root
> folder* ("New Bookmark Folder"). Probably another bug (#?), though.

That is bug 36339.

from comment 51:

> This bug is blocking bug 145941 ([Meta] All resizable/movable windows should
> remember user selected size/position). Will this bug fix those issues, or 
> should I file a separate bug for remembering size & position?

In my opinion, it should be filed as a separate bug. That way it's easier to
focus on fixing the bug.

But while I was testing my patch I noticed that the size and position of the
file bookmark dialog was saved. I didn't test it extensively, though. Maybe it
has been fixed already?

Comment 53

[Håkan Waara]

> I have seen this too, both in 1.0 branch and trunk. In this patch I set the
> height for folderbox vbox to 20em and it seems to resolve the problem. Is there
> a better way than this? I'm a shamed to admit but I don't anything about XUL.

Eek! Please *never* hardcode sizes, colors or similar into the XUL file, that
will break themes who apply their CSS to these files.  I think the size problem
is another filed bug, so let's not mess with hardcoded values and try to find
that bugzilla bug instead...

Comment 54

[Håkan Waara]

Reassign to Kalle who is working on this now.

Comment 55

[Kalle Valo]

> Please *never* hardcode sizes, colors or similar into the XUL file, that
> will break themes who apply their CSS to these files.

Ok. I got the height property from Gerv's patch, just moved it to another
element. The width property was already in there.

http://lxr.mozilla.org/seamonkey/source/xpfe/components/bookmarks/resources/addBookmark.xul#39

Should I remove the dialog's width property also?

> I think the size problem is another filed bug, so let's not mess with  
> hardcoded values and try to find that bugzilla bug instead...

That bug might be bug 123207.

Comment 56

[Håkan Waara]

I think usually we put a default width/height on the dialog, and when a user
changes the size, it will be persisted, but I don't think we *usually* do that
on widgets.

Comment 57

[Kalle Valo]

Attachment: Kalle's patch v0.2

I have changed these:

o changed order of 'New Folder' and 'Default Folder' buttons

o removed height property from vbox

o Changed 'Create in' to 'Destination'

This patch doesn't fix the size problem of the 'File Bookmark' dialog. That is
a separate issue and should not be handled here.

Comment 58

[Kalle Valo]

Attachment: File Bookmark dialog with Kalle's v0.2 patch

I had two tab windows open so 'File as group' checkbox is shown.

Comment 59

[Håkan Waara]

If possible, make the "File as Group" checkbox line up with the above textfield,
then you have r=hwaara.

Comment 60

[Kalle Valo]

> If possible, make the "File as Group" checkbox line up with the above textfield

Could you be more specific? Do you want the checkbox line up with the
'Location:' label or with the textbox for URL?

The latter case means that the checkbox just has to be moved few pixels to the
left. I don't know how to do that.

Comment 61

[Håkan Waara]

I meant for the textbox of the URL, but if that is not possible without
hardcoding a position (we don't want to that) -- no problem.

Comment 62

[Pierre Chanial]

Kulle, the inclusion of bookmarksTree.js should not be necessary.

Comment 63

[Pierre Chanial]

s/Kulle/Kalle

Comment 64

[O'Shaughnessy Evans]

Is there a way I can try out Kalle's patch with one of the nightly builds?  Thanks.

Comment 65

[Matthew T (active 1999-2002)]

Good work. However, there are a couple of things which need fixing:
1.  The `Name:' and `Address:' labels should be right-aligned, not left-aligned.
2.  `Destination:' should have accesskey `D', and `Use Default' should have
    accesskey `U'.

Comment 66

[Pierre Chanial]

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

Comment 67

[Pierre Chanial]

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

Comment 68

[Daniel Brooks [:db48x]]

Comment on attachment 84884 [details] [diff] [review]
Kalle's patch v0.2

r=db48x

Comment 69

[Mike Kowalski]

Attachment: fix

I took the original patch and fixed some bugs. I also combined this patch with
another patch, so that it fixes 2 bugs.

Comment 70

[Adam Masri]

In comment 57,

> This patch doesn't fix the size problem
> of the 'File Bookmark' dialog. That is a
> separate issue and should not be handled here.

That was bug 139591. So, reopening.

- Adam

Comment 71

[Peter Lairo]

actually, bug 139591 was marked a dupe of bug 123207. So everyone interested in
having the "File Bookmark" window remember the user-selected size (and
position?), please go to bug 123207.

Comment 72

[Kalle Valo]

Mike, your patch (attachment 90829 [details] [diff] [review]) doesn't work. There is XML Parsing error in
addBookmark.xul while choosing Bookmarks/File Bookmark from menu.

Comment 73

[Kalle Valo]

Attachment: Kalle's patch v0.3

Here's a new patch with changes from comment 65.

List of what this patch changes:

o new license block
o title renamed from 'Add Bookmark' to 'File Bookmark'
o 'Create in:' label renamed to 'Destination:'
o order of 'New Folder' and 'Use Default' buttons changed and moved
  to under of bookmark tree
o added accesskey attribute to 'Destination:' label
o `Destination:' now has accesskey `D'
o `Use Default' has accesskey `U'


Things not done:

o The `Name:' and `Address:' labels should be right-aligned, not
  left-aligned. (I tried to fix this, but I could not get pack-attribute work
  with column element. I left labels as they were.)

Comment 74

[Kalle Valo]

Attachment: File Bookmark dialog with Kalle's v0.3 patch

Comment 75

[Daniel Brooks [:db48x]]

Comment on attachment 93758 [details]
File Bookmark dialog with Kalle's v0.3 patch

I like it. I don't think the alignment of those two labels matters, really.

Comment 76

[Kalle Valo]

There's a problem with bookmarks tree. It's default vertical size is too small
and my patch v0.3 makes the problem even worse. See attachment 93760 [details] for a
screenshot and bug 146859 for details.

Comment 77

[Kalle Valo]

From comment #65:

> The `Name:' and `Address:' labels should be right-aligned, not left-aligned.

I have to disagree with this. In my opinion they should be left-aligned. By
making them right-aligned would reduce usability. I found UI specification for
dialogs
<http://www.mozilla.org/projects/ui/communicator/framework/dialog_framework/>
which says they should be right aligned, but the document's status is marked as
old. What should I do?

Proxy labels in preference window are right aligned but labels in 'Open File'
dialog are left aligned. Go figure.

Comment 78

[Pierre Chanial]

Attachment: one more screenshot...

Thanks Kalle for your help on this bug. I've been working on it today, based on
your last patch. Here is the screenshot. What's new?
- I added the description field, since a organized person that use it will want
to give a short description before forgetting about it. 
- 'Use default' button removed since when bug 36339 will be fixed, it will be
unnecessary. 
- I changed also the 'Destination' label since its access key conflicts with
'Description'.
- right alignment of the labels: if the specs say so, let's follow them for
consistency. A good thing to do would be to collect all the places in the app
where labels are left aligned and file a new bug.
- the size problem at launch and persistency fixed.
Comments are welcome!

Comment 79

[Christian :Biesinger (don't email me, ping me on IRC)]

it would be nice to add a "keyword" field, as comment 0 also suggested

Comment 80

[Pierre Chanial]

I don't think adding the keyword field is worthy:
- it's a geek feature.
- the normal user will already be aggressed by having to give one more
information with the addition of the 'Description'. In this case, he/she will
understand what this field is about and the benefit he/she will have to use it.
- the bookmark keyword is generally set after that one has realized that it
sucks so much to find the bookmark, type the url or the first letters in the url
bar, that he/she will assign a keyword to a bookmark. Well the bookmark has
already been filed.

Comment 81

[Peter Lairo]

> I don't think adding the keyword field is worthy:

Actually, it would finally make keywords discoverable, and thus novice-appreciated.

Comment 82

[Peter Lairo]

comment #78: 
> 'Use default' button removed since when bug 36339 will be fixed, it will be
unnecessary.

Actually, IIRC, the *Default* folder does not have to be the *Root* folder. I
think NC4 let you define *any* folder as the *default* folder.

Thereforwe, you may want to consider re-adding the 'Use default' button. ;)

Comment 83

[Pierre Chanial]

My plan is to save the last selected folder and even make it permanent across
sessions.

Comment 84

[Peter Lairo]

That would be a mistake. People need a "default" location. Wildly saving
bookmaks depending on where you surfed yesterday will REALLY confuse novices.
Remember, novices will not look at which folder is selected; they will just hit
[OK] (and wonder why they can never find that BM again).

Comment 85

[Matthew T (active 1999-2002)]

Comment on attachment 93931 [details]
one more screenshot...

Thanks for your work Pierre. A few things remain to be fixed:
1.  The Description field needs to be removed, since it's even less useful than
the Keyword field is.
2.  `Select a Folder:' should be `Destination:', (as in comment 0), since you
*don't* need to select a folder.
3.  The accesskey for `New Folder...' should be N, not W.
4.  The `Name' column header should be removed from the tree control, since
it's completely useless.

Comment 86

[Peter Lairo]

Keywords and Descriptions ARE useful. What better time to give a bookmark a
meaningful Description and a Keyword than when creating it? The space gained by
removing them is minimal and insignificant. I *frequently* resent having to
reopen my bookmarks to give them a Description and Keyword.

FWIW I agree with the OTHER three corrections mpt suggested.

Comment 87

[Sören 'Chucker' Kuklau (gone)]

Keyword is a very useful technique Mozilla offers, but it's hard to tell for
most users what it's for (ever found any mention of "%s" in any
Bookmarks-related dialog? I didn't).

Description is a nice idea, but makes no sense at the moment as you can*not*
read the description anywhere else than in the bookmark properties. The
description *should* be made accessible in at least one additional way; before
that, they do not have a point.

Comment 88

[Gervase Markham [:gerv]]

Remember that there is already an instant-bookmark key/menu item. This dialog is
the thinking man's bookmark creation method ;-)

Yes, description should probably be exposed elsewhere, if we decide it's worth
having at all. Removing the box would be a regression. However, given that even
a technically-advanced person will probably only have a few bookmarks with
keywords, going to the Bookmarks Manager to set those up seems reasonable. So I
don't think there should be a keywords field. (As I remember, the internal API
would also need changing to allow this.)

Gerv

Comment 89

[Ansgar Schrage]

I have to add something to comment 85. The 'Name' column header makes sense,
because it allows you to sort ascending or descending when you click on it.

Comment 90

[Antony Mawer]

I think the keyword system as it stands, while useful, is very counter-intuitive.

Yes, they're incredibly handy -- especially when used in conjunction with %s for
quick searching (eg. bug #####) and soforth. However, I find the fact that to
create these "search keywords", I have to create a bookmark, a little silly. Why
would I want to bookmark http://www.somesite.com/page?%s ? I can't click on that
link to take me anywhere useful.

Sure, keywords are useful for a quick way of accessing a page. But perhaps they
should be distinct from "Bookmarklets", search keywords, or whatever they're
called -- which might be better maintained in a dedicated dialog somewhere.

This should probably be another bug though. Just my 2 cents.

With regards to making Keywords more discoverable, I think the better way to do
this could be with some form of Wizard that steps the user through the process.
I can just imagine end users... "I have to put %s where?"

I myself found the keywords somewhat non-intuitive to set up.

I also find the fact that you have to create a bookmark somewhere for a keyword
a bit misleading -- after all, that bookmark is usually of little use when
clicked on directly.

Comment 91

[m_mozilla]

> Why would I want to bookmark http://www.somesite.com/page?%s ?
> I can't click on that link to take me anywhere useful.


Actually, very slowly, sites are begining to accomodate this.

This keyword will be of little to no utility to you, but

    set 'p' as a keyword for http://peds.wustl.edu/%s

Now you can go to places in that site like so:

    p div/gi
    p div/id
    p div/research
    p faculty
    p residency

And if you just want to go to the root of the site,

    p

will take you to an existing http://peds.wustl.edu/%s which *does* redirect to
the root of the site.


> might be better maintained in a dedicated dialog somewhere


...but you're right. We shouldn't expect everyone to start guessing where we
want to put our %s tokens, and making the referenced resources actually valid.

bug created: bug 162777
            "custom keyword queries are not bookmarks. Separate them."

-matt

Comment 92

[Kalle Valo]

I reassign this bug back to ben. I've done my share.

Comment 93

[Chris Lyon]

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

Comment 94

[Ben Goodger (use ben at mozilla dot org for email)]

Attachment: proposal for redesign.

Comment 95

[Ben Goodger (use ben at mozilla dot org for email)]

-> Jan for buffy. Jan, take a look at my latest attachment & discuss with UE.
This could be a chance to fix some issues that make Bookmarks less easy to file. 

Comment 96

[Gervase Markham [:gerv]]

Ben or Jan: the new design looks much simpler. But I have a UI question.

Does the dropdown list of folders include every folder (including sub-folders)
in the user's Bookmarks? Or does it just include top-level folders? Or is it a
tree widget disguised as a pulldown menu?

Gerv

Comment 97

[Jan Varga [:janv]]

nominating according to PRD

Comment 98

[Samir Gehani]

Nav triage team: nsbeta1+/adt2

Comment 99

[Jan Varga [:janv]]

Gerv, good question, sorry for not responding sooner.
Actually I don't know either. I need to check with Ben or Marlon

Comment 100

[Vaclav Dvorak]

So, how hard would it be to first fix bug 82721, so that bug 89001 could then be
fixed together with this one? Wouldn't it be a pity to spend time on cosmetic
details of the File Bookmark dialog, only to revisit that same dialog later when
adding the Keyword and Description fields? Personally, I don't really see what's
so very wrong with the dialog, _except_ for the missing Keyword and Description
fields. (OK, yes, I did read the description etc., I just don't think it's so
important.)

Comment 101

[Jan Varga [:janv]]

The bookmarks branch has landed.
Fixed.

Comment 102

[Chris Petersen]

Verified on the 2003-03-25-03 Mach0 OS X trunk and 2003-03-25-04 Win32 trunk builds.