Closed
Bug 586947
Opened 14 years ago
Closed 14 years ago
Places File Bookmark (Ctrl+D) needs improvement badly
Categories
(SeaMonkey :: Bookmarks & History, defect)
Tracking
(Not tracked)
RESOLVED
FIXED
seamonkey2.1b2
People
(Reporter: philip.chee, Assigned: philip.chee)
References
Details
(Keywords: regression)
Attachments
(4 files, 6 obsolete files)
40.13 KB,
image/png
|
Details | |
17.99 KB,
image/png
|
kairo
:
feedback-
|
Details |
29.59 KB,
image/png
|
Details | |
8.70 KB,
patch
|
philip.chee
:
review+
neil
:
superreview+
|
Details | Diff | Splinter Review |
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.
Assignee | ||
Comment 1•14 years ago
|
||
> 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.
Attachment #465626 -
Flags: review?(kairo)
Attachment #465626 -
Flags: feedback?(iann_bugzilla)
Assignee | ||
Comment 2•14 years ago
|
||
Question for Enn:
> 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.
Comment 3•14 years ago
|
||
(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?
Assignee | ||
Comment 4•14 years ago
|
||
> 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?
Assignee | ||
Comment 5•14 years ago
|
||
OK level="top" worked. However I have "one or more text fields" and the MDC article on <panel> recommends that top be avoided in this situation.
Comment 6•14 years ago
|
||
That's odd. Is this Windows only?
Assignee | ||
Comment 7•14 years ago
|
||
Sorry I'm on windows 7, I can't test on other platforms. The docs say that "parent" is the default on Windows so, without specifying a level= how do I get dragging to work?
Comment 8•14 years ago
|
||
The default level is 'floating' if a titlebar is used, so it should work. A testcase would be good here and I could investigate further.
Assignee | ||
Comment 9•14 years ago
|
||
Attaching screenshot of WIP
Comment 10•14 years ago
|
||
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!
Assignee | ||
Comment 11•14 years ago
|
||
> 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
Comment 12•14 years ago
|
||
(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.
Assignee | ||
Comment 13•14 years ago
|
||
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.
Assignee | ||
Comment 14•14 years ago
|
||
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.
Attachment #465626 -
Attachment is obsolete: true
Attachment #466364 -
Flags: review?(iann_bugzilla)
Attachment #465626 -
Flags: review?(kairo)
Attachment #465626 -
Flags: feedback?(iann_bugzilla)
Comment 16•14 years ago
|
||
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 17•14 years ago
|
||
Well I wasn't going to post (here), just to post a "me too", but have to say, I agree wholeheartedly with Jens. (My comments, which basically mirror, are in mozillazine & cZ.)
Comment 18•14 years ago
|
||
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.
Comment 19•14 years ago
|
||
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.
Depends on: SMPlacesBMarks
Comment 20•14 years ago
|
||
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.
Attachment #466364 -
Flags: review?(iann_bugzilla) → review+
Comment 21•14 years ago
|
||
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.
Comment 22•14 years ago
|
||
(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.
Comment 23•14 years ago
|
||
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.
Comment 24•14 years ago
|
||
And by the way, on Windows, going from left to right, we (the users) want the "OK" button before the "Cancel" button, not the vice versa.
Comment 25•14 years ago
|
||
Yep, the same dialog in Firefox, except the buttons are in the right order (at least in 3.6.8). Aliens go home.
Assignee | ||
Comment 26•14 years ago
|
||
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);
+}
Attachment #466364 -
Attachment is obsolete: true
Attachment #469472 -
Flags: superreview?(neil)
Comment 27•14 years ago
|
||
Comment on attachment 466367 [details]
Screenshot of Patch 1.1.
OK, as people don't listen to my comments, I'll f- this.
Attachment #466367 -
Flags: feedback?(kairo) → feedback-
Comment 28•14 years ago
|
||
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!
Assignee | ||
Comment 29•14 years ago
|
||
Requested by therube
Comment 30•14 years ago
|
||
(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.
Comment 31•14 years ago
|
||
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).
Comment 32•14 years ago
|
||
(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 33•14 years ago
|
||
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
Attachment #469472 -
Flags: superreview?(neil) → superreview-
Assignee | ||
Comment 34•14 years ago
|
||
> 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.
Attachment #469472 -
Attachment is obsolete: true
Attachment #476672 -
Flags: review?(neil)
Comment 35•14 years ago
|
||
Comment on attachment 476672 [details] [diff] [review]
Patch v2.0 Use the bookmark properties dialog.
You forgot to actually hook up your code to the UI...
>+ if ("foldertree" in dialogInfo) {
>+ if (this._element("folderTreeRow").collapsed) {
>+ setTimeout(function() gEditItemOverlay.toggleFolderTreeVisibility(), 10);
I don't like the tree being uncollapsed by default, especially on a timeout.
>+ var parent = aParent != undefined ?
>+ aParent : PlacesUtils.bookmarksMenuFolderId;
aParent || PlacesUtils.bookmarksMenuFolderId>+ PlacesUIUtils.showAddBookmarkAsUI(uri, title, description, insertPoint);
Can't you use showAddBookmarkUI instead?
>+ info.hiddenRows = aHiddenRows ? aHiddenRows : ["loadInSidebar"];
[aHiddenRows ||]
>+#editBMPanel_tagsSelector {
>+ max-height: 5em;
>+}
>+
>+#editBMPanel_tagsSelector > listrows > listboxbody {
>+ display: inline-block;
>+}
Not really relevant to this bug?
Attachment #476672 -
Flags: review?(neil)
Assignee | ||
Comment 36•14 years ago
|
||
> neil@parkwaycc.co.uk 2010-09-26 13:15:49 PDT
>
> Comment on attachment 476672 [details] [diff] [review]
> Patch v2.0 Use the bookmark properties dialog.
>
> You forgot to actually hook up your code to the UI...
Fixed.
> >+ if ("foldertree" in dialogInfo) {
> >+ if (this._element("folderTreeRow").collapsed) {
> >+ setTimeout(function() gEditItemOverlay.toggleFolderTreeVisibility(), 10);
> I don't like the tree being uncollapsed by default, especially on a timeout.
Removed.
> >+ var parent = aParent != undefined ?
> >+ aParent : PlacesUtils.bookmarksMenuFolderId;
> aParent || PlacesUtils.bookmarksMenuFolderId
Fixed.
> >+ PlacesUIUtils.showAddBookmarkAsUI(uri, title, description, insertPoint);
> Can't you use showAddBookmarkUI instead?
Now using showAddBookmarkUI().
> >+ info.hiddenRows = aHiddenRows ? aHiddenRows : ["loadInSidebar"];
> [aHiddenRows ||]
Fixed by removing this.
> >+#editBMPanel_tagsSelector {
> >+ max-height: 5em;
> >+}
> >+
> >+#editBMPanel_tagsSelector > listrows > listboxbody {
> >+ display: inline-block;
> >+}
> Not really relevant to this bug?
I'll file a follow up bug.
Attachment #476672 -
Attachment is obsolete: true
Attachment #482885 -
Flags: review?(neil)
Comment 37•14 years ago
|
||
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";
>+ var features = "centerscreen,chrome,modal,resizable=yes";
Why this change?
Assignee | ||
Comment 38•14 years ago
|
||
>>- 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?
Assignee | ||
Comment 39•14 years ago
|
||
> 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.
Comment 40•14 years ago
|
||
(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.
Assignee | ||
Comment 41•14 years ago
|
||
> 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.
Attachment #482885 -
Attachment is obsolete: true
Attachment #483976 -
Flags: review?(neil)
Attachment #482885 -
Flags: review?(neil)
Comment 42•14 years ago
|
||
Comment on attachment 482885 [details] [diff] [review]
Patch v2.1 fix nits.
The resizability issue is hurting my brain. Can we leave that bit for now?
Comment 43•14 years ago
|
||
Description and keyword fields are missing.
Assignee | ||
Comment 44•14 years ago
|
||
> 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.
Assignee | ||
Comment 45•14 years ago
|
||
> 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 46•14 years ago
|
||
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.
Attachment #482885 -
Attachment is obsolete: false
Attachment #482885 -
Flags: review+
Updated•14 years ago
|
Attachment #483976 -
Flags: review?(neil) → review-
Assignee | ||
Comment 47•14 years ago
|
||
>>- 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.
Removed.
Attachment #482885 -
Attachment is obsolete: true
Attachment #483976 -
Attachment is obsolete: true
Attachment #484649 -
Flags: superreview?(neil)
Attachment #484649 -
Flags: review+
Comment 48•14 years ago
|
||
Comment on attachment 484649 [details] [diff] [review]
Patch v2.1.1 Without resizeable changes. r=Neil
Thanks.
Attachment #484649 -
Flags: superreview?(neil) → superreview+
Assignee | ||
Comment 49•14 years ago
|
||
Pushed to comm-central
http://hg.mozilla.org/comm-central/rev/428648891029
Resolving as FIXED. Follow-up bugs will be filed for the remaining issues.
Status: ASSIGNED → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
Assignee | ||
Updated•14 years ago
|
Target Milestone: --- → seamonkey2.1b2
Comment 50•14 years ago
|
||
Seesh. Now I'll need to file a bug about the ugly large width of this panel. :(
Comment 51•14 years ago
|
||
(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.
You need to log in
before you can comment on or make changes to this bug.
Description
•