Separate out the view model from a nsNavHistoryResult

RESOLVED FIXED in Firefox 2 alpha2

Status

()

Firefox
Bookmarks & History
P1
normal
RESOLVED FIXED
12 years ago
8 years ago

People

(Reporter: Brett Wilson, Assigned: Brett Wilson)

Tracking

({fixed1.8.1})

Trunk
Firefox 2 alpha2
fixed1.8.1
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(3 attachments)

161.85 KB, patch
Ben Goodger (use ben at mozilla dot org for email)
: review+
Details | Diff | Splinter Review
11.23 KB, patch
Ben Goodger (use ben at mozilla dot org for email)
: review+
Details | Diff | Splinter Review
2.94 KB, patch
Annie Sullivan
: review+
Details | Diff | Splinter Review
(Assignee)

Description

12 years ago
Currently nsNavHistoryResult implements a treeview and notifies the associated tree of things being added, deleted, or changed. This kind of sucks because other non-tree uses (like the toolbar) don't know what's changed.

There should be an in-between view interface. One standard component would be a tree view that implements the current functionality. Other users such as the toolbar would implement their own.
(Assignee)

Updated

12 years ago
Priority: -- → P1
Target Milestone: --- → Firefox 2 alpha2
(Assignee)

Comment 1

12 years ago
Created attachment 215141 [details] [diff] [review]
Backend patch
Attachment #215141 - Flags: review?(annie.sullivan)
(Assignee)

Comment 2

12 years ago
Created attachment 215142 [details] [diff] [review]
Frontend patch
Attachment #215142 - Flags: review?(annie.sullivan)
(Assignee)

Comment 3

12 years ago
Created attachment 215144 [details] [diff] [review]
Build patch

Sorry, I forgot the browser/components/build stuff in the previous patches.
Attachment #215144 - Flags: review?(annie.sullivan)

Comment 4

12 years ago
Comment on attachment 215142 [details] [diff] [review]
Frontend patch

Weren't there supposed to be changes to toolbar.xml as well, to move it from always rebuilding when its observer gets a notification to using the view to update its UI?

Updated

12 years ago
Attachment #215144 - Flags: review?(annie.sullivan) → review+
(Assignee)

Comment 5

12 years ago
(In reply to comment #4)
> (From update of attachment 215142 [details] [diff] [review] [edit])
> Weren't there supposed to be changes to toolbar.xml as well, to move it from
> always rebuilding when its observer gets a notification to using the view to
> update its UI?

You or I can do that separately. The toolbar will keep working the way it is in the meantime.

Comment on attachment 215141 [details] [diff] [review]
Backend patch

r=ben@mozilla.org
Attachment #215141 - Flags: review?(annie.sullivan) → review+
Comment on attachment 215142 [details] [diff] [review]
Frontend patch

r=ben@mozilla.org
Attachment #215142 - Flags: review?(annie.sullivan) → review+
(Assignee)

Comment 8

12 years ago
On branch and trunk.
Status: NEW → RESOLVED
Last Resolved: 12 years ago
Keywords: fixed1.8.1
Resolution: --- → FIXED

Comment 9

12 years ago
Looks like failing to change the getResult() call to a getResultView() in tree.xml:553 during this change broke the Add Bookmark dialog.

Comment 10

12 years ago
OK, I put a quick fix for the breakage mentioned above into my patch for bug 330192, which I just landed on branch & trunk.
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.