From http://forums.mozillazine.org/viewtopic.php?p=9744353#p9744353
I find File Bookmark (Ctrl+D) to be FAR less useful.
1) Non-movable dialog displays upper right of browser window. I often have my download manager's capture window at just that location. So the one overlays the other.
2) dialog is small
3) dialog is hardly conducive to editing on the fly
4) even if you were to edit, you can only edit the Name:
5) you CANNOT manipulate the Location: at all
IMO the entire reason to use Ctrl+D is to be able to quickly & easily edit a bookmark to be saved. You can no longer do that. And if you cannot do that, then why even use Ctrl+D. You can just use Ctrl+Shift+D (Bookmark This Page) & be done with it.
For me, just about every time I Ctrl+D, it ends up being a 2-step (or more) process. First Ctrl+D, then I need to Ctrl+B (to open Bookmark Manager) so that I can then edit the entry to where I wanted it to be in the first place.
Example: http://forums.informaction.com/viewtopic.php?f=6&t=4157&sid=c07902855734745a3e8802d7cf4eef35
I'm viewing this thread. Hmm, great post, think I'll bookmark it. Looky there, the URL has that dumb sid= tacked on the end. Now that shouldn't be there. Ctrl+D. Uh? Duh? How do I edit the Location: ? How do I get rid of sid?
Screw it. Ctrl+D done. (Or not.) Ctrl+B, find the bookmark I just saved, then edit it. Dumb.
Storing username/password in the Location:. I do it all the time. Not the smartest thing perhaps, but it works for me. Often I need to reference or copy the un/pw.
Ctrl+D. There it is. Location: shows, http://therube:trythis@forums.mozillazine.org/. Just what I wanted to see. Now very simple for me to copy/paste 't h e r u ...". Ooops, wait a minute, Location: is no longer there? I can't see my un/pw. Now I have to Ctrl+B.

Created attachment 465626[details][diff][review]
WIP Patch 1.0. proposed enhancement.
> 1) Non-movable dialog displays upper right of browser window. I often have my
> download manager's capture window at just that location. So the one overlays
> the other.
> 2) dialog is small
> 3) dialog is hardly conducive to editing on the fly
> 4) even if you were to edit, you can only edit the Name:
> 5) you CANNOT manipulate the Location: at all
This patch fixes issues #2 to #5.
I don't know why the panel isn't draggable. Or rather I can drag the panel via the titlebar but once I release the mouse it jumps back to the original position.
Also see Firefox Bug 418864 (Bookmark contextual dialog is not resizable) for a long flame war when users upgrading ran into the new bookmarks panel.

(In reply to comment #2)
> > I don't know why the panel isn't draggable. Or rather I can drag the panel via
> > the titlebar but once I release the mouse it jumps back to the original
> > position.
That's the correct behaviour for a panel with level="parent". The panel maintains its anchored position relative to its parent. For this reason, using a titlebar with level="parent" isn't particularly useful.
You should never change the level attribute unless you know why you are doing so. Do you?

> You should never change the level attribute unless you know why you are doing
> so. Do you?
Initially I didn't have any level attribute and the panel wasn't draggable via the titlebar with the same symptoms. So with or without level the panel isn't draggable at the moment. Any suggestions?

> Ouch, that screenshot looks like yet another of those heavy and large windows
> I'm so glad we are trying to get rid of with the new approach!
It's a regression (multiple regressions). Didn't you read comment 0? Firefox has done a great job dumbing down the UI, we don't need to compete at that level. Also do read Bug 418864. Do we want a repeat of that?
Phil

(In reply to comment #11)
> Firefox has done a great job dumbing down the UI
In this case, I call that cleaning up UI. And both the Firefox places team and me know it can be even smoother and nicer (in this case, I'd e.g. really like to see use of the arrow panels or what they are called), but IMHO the current panel is a huge improvement over the ugly dialog we had before.
> Also do read Bug 418864. Do we want a repeat of that?
Ah, good, something to dupe to! :P
Seriously, I didn't even look at the patch, but the screenshot is IMHO ugly as hell and I would veto that being the default UI, with one reason being that it's way too large, for example, one other being the title.

It sure looks much better than the tiny squidged up Firefox panel you copied over! Many Firefox users were dismayed at the loss of usability and functionality. Simplifying is one thing, removing needed functionality is just silly.

Created attachment 466364[details][diff][review]
Patch v1.1
Changes since the last patch:
1. More compact UI: Folder tree is collapsed by default. Initial width is 32em.
2. Moved the anchor back to the right of the urlbar.
3. Resizer moved to bottom left.
4. The tags in the tag selector box are now inline to make the expanded tag box more compact.
<panel id="editBookmarkPanel"
+ close="true"
+ level="top"
Temporarily set level to top in order to test draggability. Will remove later.

I also feel this is a regression (just overlooked it until now). I don't think each and every aspect of Philip's proposal is really needed but this is what bugs me about the new dialog:
- position (top right? Are you kidding me? I have a big screen!)
- not movable
- not resizable
- folder list is collapsed by default and state changes are not saved
- Tags expander opens a huge space for what I consider to be rarely used to that extent
- cannot define a Keyword anymore (I use that heavily and see no sense in having to hunt down the newly created bookmark just to set it!)
- cannot change the address
What I care less about:
- Description. I'm not using that field at all, and doubt that many do.
- Load this bookmark in the sidebar. Seems to be advanced.
Positive:
- allows to remove the bookmark if it already exists (needs the star icon or similar to unlock its full potential, though)
My current File Bookmark in SeaMonkey 2.0 is about 60% of my 22" screen (width + height) so that I can see a large part of the bookmarks folder structure. I like that it's an ordinary window dialog which can be repositioned, resized, and which keeps that state across invocations.
If this turns out to become a matter of preference we should probably either implement an alternative File Bookmark dialog or create an extension. I'd prefer the former since I guess many people will be just as annoyed as Philip and me about this and having to maintain an extension is always an extra burden.
(In reply to comment #15)
> Created attachment 466367[details]
> Screenshot of Patch 1.1.
>
> More compact UI.
Having the resizer down left looks just wrong, at least for non-RTL locales.

Comment on attachment 466367[details]
Screenshot of Patch 1.1.
That's definitely better, though I still am not too fond of the title bar, I think making it an arrow panel and adding the bookmarks icon to point the arrow to should do it. Also, the title itself is probably not ideal as IIRC, we only show the panel right now when adding a bookmark, for editing, you get a separate properties window right now, AFAIK.

Filed bug 589601 and a patch for the bookmarking button in the URL bar, this together with arrow panels (bug 554937) should make this panel feel much better.
Together with your work on at least the resizing, possibly more, I hope a lot more people will be able to like this panel.
For the resizing, we should make sure the size persists across calls, so those people who want it larger do always get it larger, and those who want it nicely compact will have it kept compact.

Comment on attachment 466364[details][diff][review]
Patch v1.1
>- gEditItemOverlay.initPanel(this._itemId,
>- { hiddenRows: ["description", "location",
>- "loadInSidebar", "keyword"] });
>+ gEditItemOverlay.initPanel(this._itemId);
I'm happy with keyword and location to be shown, not sure about the other two, if we're trying to replicate the old dialog then should the destination be expanded (unhidden) as well?
>+ //gEditItemOverlay.toggleFolderTreeVisibility();
Nit: not needed?
No changes needed for css of Mac?
r=me with those addressed.

Few thoughts.
While I don't use the Description field, some sites pre-populate it, so it would be nice to know that potentially that data could be saved in the bookmark too.
The Remove Bookmark button, does it belong there at the top like that? Would seem to be more appropriate to be bottom, left justified (& grayed out till such time as the bookmark has actually been save)?
Duplicates. There needs to be some mechanism to easily save duplicate bookmarks from within a File Bookmarks dialog.

(In reply to comment #21)
> While I don't use the Description field, some sites pre-populate it, so it
> would be nice to know that potentially that data could be saved in the bookmark
> too.
Write an add-on for that, I guess that 98% of users probably never want or need the description field.
> The Remove Bookmark button, does it belong there at the top like that?
So that you don't need too large mouse movements when you want to remove the bookmark for the site you visited. Click the bookmark button, click the remove button appearing right below it, done.
> Duplicates. There needs to be some mechanism to easily save duplicate
> bookmarks from within a File Bookmarks dialog.
I disagree, the file/edit panel doesn't need that, and the Bookmarks Manager, as well as proxy-icon-drag functionality allow that to be done for the rare cases where it's needed.
If you want the function for filing dupes in that panel, write an add-on.

I was pretty happy with the old dialog (pre-Places one). I would be the happiest man in the world if the dialog had the "Description" field as in the bookmark properties dialog in the Bookmark Manager UI. Then when I saw the new dialog for the first time, my first thought was: OMFG. WHAT. THE. ****. IS. THIS?! Unmovable, unresizable, disappearing on a mouse scroll/outside click, even without the "Location" field? Then I wondered: is it the same thing Firefox users are doomed to deal with every day, or is this brilliant solution exclusive for SeaMonkey? Now I'm going to install Firefox to find this out.

Created attachment 469472[details][diff][review]
Patch v1.2.
This patch:
1. makes the panel resizable and draggable.
1a. The resizer is in the bottom left corner because KaiRo didn't like it in the bottom right corner.
1b. The titlebar exists to make the panel draggable. Enn is working on a trunk patch to make panels draggable without a titlebar. Once that works we can remove the titlebar. In the meantime I've reclaimed some vertical space by moving the "Title" label into the titlebar.
2. The expanded Tags selector row is now more compact using an inline style.
3. There are no mac specific styles in this patch because editBookmarkOverlay.css isn't forked.
Changes since the previous patch:
1. The description and loadInSidebar rows are hidden.
2. Removed the close button on the panel title.
3. The panel size is persisted. I couldn't persist the position because the panel popup code forces the panel to anchor to the right edge of the urlbar.
4. display: inline; here works.
> +#editBMPanel_tagsSelector > listrows > listboxbody {
+ display: inline;
+}
But causes the following error. I forgot what the fix for this is.
Warning: XUL box for listrows element contained an inline listboxbody child, forcing all its children to be wrapped in a block.
Source file: chrome://navigator/content/navigator.xul
5. I don't know why the background url isn't working. The DOMi shows tha the computed style is the rtl resizer but visually it is still the ltr resizer image.
> +/* Toolkit stripe is missing a rule for bottomleft resizers */
+resizer.bottomleft {
+/* XXXRatty: this doesn't work! background: url ("chrome://global/skin/icons/resizer-rtl.png") no-repeat; */
> + -moz-transform: scaleX(-1);
+}

Comment on attachment 469472[details][diff][review]
Patch v1.2.
>+ noautohide="true"
>+ titlebar="normal"
I still disagree with both those lines strongly, including the implied draggability.
I like the resizer and persisting of size, though.-
> #editBookmarkPanelContent {
> min-width: 23em;
>+ width: 32em;
> }
As it can be resized anyhow, I don't think there's any reason to change the default.
>+ -moz-transform: scaleX(-1);
Nice solution!

(In reply to comment #18)
> That's definitely better, though I still am not too fond of the title bar, I
> think making it an arrow panel and adding the bookmarks icon to point the arrow
> to should do it. Also, the title itself is probably not ideal as IIRC, we only
> show the panel right now when adding a bookmark, for editing, you get a
> separate properties window right now, AFAIK.
I think Bookmarks/File Bookmark... should open the properties window.
On the other hand, the fast bookmarking button probably should use this panel.

It would increase usability/discoverability to have useful default text in the tags and keyword fields. Firefox has the text 'Separate tags with commas' in the tags field (Some might otherwise assume space is the separator as I did). I propose 'For quick access from address bar' as the default text for the keyword field.
I hope not to crush Kairo's hope for a clean interface with the next proposal :)
This assumes that the Folder and Tags entries do not persist their expanded state. Would it be possible to have the expand/hide button become a split button or add a small button just to the right of it when in the expanded state to pin it in the expanded state? This would allow people who feel crippled by the simplified default view control without cluttering the view for people who expand those entries occasionally (without being pinned, the entry would be in the hidden state next time the dialog is accessed).

(In reply to comment #31)
> It would increase usability/discoverability to have useful default text in the
> tags and keyword fields. Firefox has the text 'Separate tags with commas' in
> the tags field (Some might otherwise assume space is the separator as I did).
> I propose 'For quick access from address bar' as the default text for the
> keyword field.
Actually, what they have there is probably a placeholder text, detectable as a user by being displayed in a different ("deactivated") color than normal text entries there, and by going away when you click in the textbox. If it's that and we are missing those, that's surely a bug and should be filed independently.
The subject of the bug right here is largely misleading as it implies to be a "do everything" bug, which are ones we should not have. Every bug should have a clear, measurable target. So, please, even if this stays due to the interesting and probably good work being done here, file anything going farther than the patch here as additional followup bugs with very specific and clear measurable targets.

Comment on attachment 469472[details][diff][review]
Patch v1.2.
I think the panel is almost* right for the URLbar icon case (bug 589601) and should stay that way. But we need the full add bookmark UI somewhere, and so far my best idea is to make Ctrl+D open the New Bookmark dialog.
* Will follow up in a separate bug

Created attachment 476672[details][diff][review]
Patch v2.0 Use the bookmark properties dialog.
> I think the panel is almost* right for the URLbar icon case (bug 589601) and
> should stay that way. But we need the full add bookmark UI somewhere, and so
> far my best idea is to make Ctrl+D open the New Bookmark dialog.
Take 2: Use the bookmark properties dialog.
> + if ("foldertree" in dialogInfo) {
> + if (this._element("folderTreeRow").collapsed) {
> + setTimeout(function() gEditItemOverlay.toggleFolderTreeVisibility(), 10);
I don't know why a setTimeout() is needed here but otherwise I get a "gEditItemOverlay is null" which doesn't make any sense as it is already defined.
> <tree id="editBMPanel_folderTree"
> class="placesTree"
> type="places"
> + treelines="true"
Turn on treelines for the folder picker folder tree. I still haven't found a way of making the folder tree flex when the dialog is resized despite liberally sprinkling "flex" all over.
> + var dialogURL = "chrome://communicator/content/bookmarks/bm-props.xul";
> + var features = "centerscreen,chrome,modal,resizable=yes";
> + this._getCurrentActiveWin().openDialog(dialogURL, "", features, info);
> + return ("performed" in info && info.performed);
I didn't use return this._showBookmarkDialog() here as I wanted the dialog to be resizable and to use the full dialog instead of the minimal dialog. I could have modified _showBookmarkDialog() but this would have affected other callers.

>>- var features;
>>- if (aMinimalUI)
>>- features = "centerscreen,chrome,modal,resizable=yes";
>>- else
>>- features = "centerscreen,chrome,modal,resizable=no";
>>+ var features = "centerscreen,chrome,modal,resizable=yes";
> Why this change?
As it stands the minimal UI is resizable but the full UI isn't and we are calling the full UI here. One of the complaints that led to this bug is that the addbookmark dialog/panel isn't resizeable. I thought to make the full UI resizable. This affects showAddLivemarkUI(), showItemProperties(), and showAddFolderUI() as well.
I could add a third parameter to _showBookmarkDialog() to force resizability if you perfer?

> I could add a third parameter to _showBookmarkDialog() to force resizability if
> you perfer?
With the File Bookmark is resizeable, since the UI is shared, showAddLivemarkUI(), showItemProperties(), and showAddFolderUI(), these other dialogs will open with the persisted width of the File Bookmark dialog with no way of resizing if it is too squished.

(In reply to comment #38)
> As it stands the minimal UI is resizable but the full UI isn't and we are
> calling the full UI here. One of the complaints that led to this bug is that
> the addbookmark dialog/panel isn't resizeable. I thought to make the full UI
> resizable. This affects showAddLivemarkUI(), showItemProperties(), and
> showAddFolderUI() as well.
>
> I could add a third parameter to _showBookmarkDialog() to force resizability if
> you perfer?
Given bug 580662 comment #12 (right at the end) maybe aMinimalUI should be renamed to aResizable?
Possibly unrelated:
The old bookmark dialog was clever and only persisted the size of the flexible elements. So if they were hidden then the dialog would default to normal size.

Created attachment 483976[details][diff][review]
Patch v2.2 use aResizeable instead of aMinimalUI.
> Given bug 580662 comment #12 (right at the end) maybe aMinimalUI should be
> renamed to aResizable?
Ooo. Yes. Fixed.
> Possibly unrelated:
> The old bookmark dialog was clever and only persisted the size of the flexible
> elements. So if they were hidden then the dialog would default to normal size.
Urm. I would prefer to do that in a follow-up bug if you don't mind. Or rather someone else can. Places bookmarks gives me a headache.

> The resizeability issue is hurting my brain. Can we leave that bit for now?
In v2.2 the only thing I did for _showBookmarkDialog() was to rename aMinimalUI to aResizable. Is that OK? Or do you mean to keep the CTRL-D dialog non-resizeable?

Comment on attachment 482885[details][diff][review]
Patch v2.1 fix nits.
>- var features;
>- if (aMinimalUI)
>- features = "centerscreen,chrome,modal,resizable=yes";
>- else
>- features = "centerscreen,chrome,modal,resizable=no";
>- this._getCurrentActiveWin().openDialog(dialogURL, "", features, aInfo);
>+ var features = "centerscreen,chrome,modal,resizable=yes";
Sorry, but I think the resizability issue is too complicated to include in this bug, so I'd prefer to go with this patch, but without this resizable change.

(In reply to comment #44)
> > Description and keyword fields are missing.
> Lets get this bug landed and decide on those fields in a followup bug. Neil has
> enough problems reviewing the current patches.
There's already a bug for that: 89001. It was erroneously closed. Please REOPEN it.

CC: Peter

You need to log in
before you can comment on or make changes to this bug.