Size Add Bookmark window and Bookmark Properties window not remembered after a restart

RESOLVED DUPLICATE of bug 357316

Status

()

P3
normal
RESOLVED DUPLICATE of bug 357316
13 years ago
9 years ago

People

(Reporter: ria.klaassen, Assigned: jminta)

Tracking

Trunk
Firefox 3
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [Fx2-parity])

Attachments

(1 obsolete attachment)

They are both called Bookmark Properties window BTW.
(Reporter)

Updated

13 years ago
Summary: Size Add Bookmarks window and Bookmark Properties windows not remembered after a restart → Size Add Bookmark window and Bookmark Properties windows not remembered after a restart
(Reporter)

Updated

13 years ago
Summary: Size Add Bookmark window and Bookmark Properties windows not remembered after a restart → Size Add Bookmark window and Bookmark Properties window not remembered after a restart

Updated

13 years ago
Assignee: nobody → joe
Priority: -- → P3
Target Milestone: --- → Firefox 2 beta1

Updated

13 years ago
Whiteboard: swag 1
Depends on: 331717
Assignee: mozilla → nobody
Blocks: 357316
OS: Windows XP → All
Hardware: PC → All
Whiteboard: swag 1 → [Fx2-parity]
Target Milestone: Firefox 2 beta1 → Firefox 3
(Assignee)

Comment 1

12 years ago
Created attachment 255042 [details] [diff] [review]
persist attributes

Patch adds persist to the window
Assignee: nobody → jminta
Status: NEW → ASSIGNED
Attachment #255042 - Flags: review?(gavin.sharp)
On IRC, Gavin asked about whether it makes sense to persist size and position given that the one dialog is used for both adding and editing bookmark properties.

On the branch, neither size or position are persisted when editing bookmarks on any platform. When adding bookmarks, width is persisted on win/mac/linux, but not height on any. Position is persisted only on win/linux. What a mess.

I think the approach in Places improves this: A single dialog handles all actions, and any persisted attributes will apply consistently across all instances of it.
Comment on attachment 255042 [details] [diff] [review]
persist attributes

this fix looks reasonable to me.  mano / gavin, what do you think?

Note, we already persist these attribute in moveBookmarks.xul (in places).
Attachment #255042 - Flags: review?(mano)
Comment on attachment 255042 [details] [diff] [review]
persist attributes

I don't think we should persist the height given that various element may be hidden.
Attachment #255042 - Flags: review?(mano) → review-
(In reply to comment #2)
> I think the approach in Places improves this: A single dialog handles all
> actions, and any persisted attributes will apply consistently across all
> instances of it.

The reason I though it was weird was that to the user, it isn't "a single dialog". The "add bookmark" and "edit bookmark" dialogs, while similar, have different titles and different uses, so having the implementation detail that they're actually the same dialog be exposed to the user through size/position persistence might be confusing, or even worse annoying. I can imagine a scenario where users want the "Add Bookmark" dialog to be large so that they can see the whole treeview, while keeping the "Edit Bookmark" dialog small, because it has no treeview. Maybe that's a bit contrived, and perhaps I'm overthinking this, but it didn't really "feel" right when I tested this patch.

That being said, the only other options that I can see are WONTFIX (which is not really an option - users who resize the dialogs do it for a reason, and there's nothing more annoying than having it not persist) or splitting these two dialogs. I don't really know how much work is involved in splitting the dialogs, or whether that really makes sense given the tradeoffs, but perhaps we should consider it? Maybe we could move the common code to a shared JS file?

What do you guys think?
(In reply to comment #7)
> Or we can persist them manually.

How would that work? The two windows would have the same ID, so the last persisted value would apply to the next opened window, regardless of whether we persisted it manually or not.
Comment on attachment 255042 [details] [diff] [review]
persist attributes

Fixed by bug 357316.
Attachment #255042 - Attachment is obsolete: true
Attachment #255042 - Flags: review?(gavin.sharp)
Status: ASSIGNED → RESOLVED
Last Resolved: 12 years ago
Resolution: --- → DUPLICATE
Duplicate of bug: 357316
Bug 451915 - move Firefox/Places bugs to Firefox/Bookmarks and History. Remove all bugspam from this move by filtering for the string "places-to-b-and-h".

In Thunderbird 3.0b, you do that as follows:
Tools | Message Filters
Make sure the correct account is selected. Click "New"
Conditions: Body   contains   places-to-b-and-h
Change the action to "Delete Message".
Select "Manually Run" from the dropdown at the top.
Click OK.

Select the filter in the list, make sure "Inbox" is selected at the bottom, and click "Run Now". This should delete all the bugspam. You can then delete the filter.

Gerv
Component: Places → Bookmarks & History
QA Contact: places → bookmarks
You need to log in before you can comment on or make changes to this bug.