Closed
Bug 412988
Opened 17 years ago
Closed 17 years ago
Wrong viewIndex after lazy changes of title
Categories
(Firefox :: Bookmarks & History, defect)
Firefox
Bookmarks & History
Tracking
()
RESOLVED
FIXED
Firefox 3 beta3
People
(Reporter: ondrej, Assigned: ondrej)
References
Details
Attachments
(2 files)
(deleted),
patch
|
dietrich
:
review+
mtschrep
:
approval1.9+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
dietrich
:
review+
mtschrep
:
approval1.9+
|
Details | Diff | Splinter Review |
When a page is added to a history query node sorted by title, it is inserted often on wrong place, because it uses part of the query string as sort key, instead of using the title. Title is lazily updated later and item is resorted.
I have observed during implementation of bug 385245 in "By Date and Site" view, that a page belonging to one site gets added to a wrong one.
Initial insert is:
[-] Today
[-] SiteA
- http://sitea.com/blog "blog"
- http://sitea.com/about "SiteA::About"
[-] SiteB
- "SiteB::About"
Temporary title "blog" is less than "SiteA::About" so it is initially inserted on position 0 inside of SiteA (viewIndex=2). "SiteA::About" has viewIndex=3.
Now title gets updated to "SiteA::Blog" and the code calculates new position inside of "SiteA" which is 1 (2nd item). Now the old algorithm would calculate the new viewIndex for "SiteA::Blog" as viewIndex of "SiteA::About" + 1. That is 4. This would place it to SiteB. So the fix is that if we detect, that we are swapping the elements inside of one parent, we need to subtract one, because "SiteA::About" will be moving by one up.
Attachment #297814 -
Flags: review?(dietrich)
Assignee | ||
Updated•17 years ago
|
Assignee: nobody → ondrej
Comment 1•17 years ago
|
||
Comment on attachment 297814 [details] [diff] [review]
Fix order when title gets updated
this change looks good, r=me.
Can you please add a comment here on the bug w/ clear STR for QA verification?
Attachment #297814 -
Flags: review?(dietrich) → review+
Updated•17 years ago
|
OS: Windows XP → All
Hardware: PC → All
Target Milestone: --- → Firefox 3
Comment 2•17 years ago
|
||
Comment on attachment 297814 [details] [diff] [review]
Fix order when title gets updated
drivers: low impact fix for a live-update node ordering scenario.
Attachment #297814 -
Flags: approval1.9?
Updated•17 years ago
|
Summary: Wrong visitIndex after laze changes of title → Wrong visitIndex after lazy changes of title
Updated•17 years ago
|
Summary: Wrong visitIndex after lazy changes of title → Wrong viewIndex after lazy changes of title
Assignee | ||
Comment 3•17 years ago
|
||
Steps To Reproduce
------------------
Instruction - wait after each visit for changes in the sidebar. Expand all nodes that will appear.
1) Clear Browsing History.
2) Open History sidebar.
3) View by Date and Site.
4) Visit http://www.mozilla.org/
5) Visit http://winscp.net/
6) Visit http://winscp.net/eng/docs/protocols
Now you should see both WinSCP pages to be inside of "winscp.net" node. Before this patch, the page http://winscp.net/eng/docs/protocols with title "WinSCP :: Supported Transfer Protocols" would land in "www.mozilla.org" node. You should try at least 3 or 5 times. This was not always visible.
Updated•17 years ago
|
Attachment #297814 -
Flags: approval1.9? → approval1.9+
Updated•17 years ago
|
Keywords: checkin-needed
Comment 4•17 years ago
|
||
Checking in browser/components/places/content/treeView.js;
/cvsroot/mozilla/browser/components/places/content/treeView.js,v <-- treeView.js
new revision: 1.29; previous revision: 1.28
done
Status: NEW → RESOLVED
Closed: 17 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: Firefox 3 → Firefox 3 M11
Assignee | ||
Comment 5•17 years ago
|
||
I was looking at bug 409676 and found problems with sorting on unexpected place, analysis showed, it has been caused by this bug, so I'm reopening it.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Assignee | ||
Comment 6•17 years ago
|
||
At the moment sorting a bookmark folder with items: "1", "2" and "3" leads to "1", "3", "2". This patch fixes this for situations when viewIndex is not defined (item has been removed and is being added again).
Updated•17 years ago
|
Attachment #299225 -
Flags: review?(dietrich)
Updated•17 years ago
|
Flags: blocking-firefox3?
Updated•17 years ago
|
Attachment #299225 -
Flags: review?(dietrich) → review+
Comment 7•17 years ago
|
||
we need a comprehensive series of litmus (or preferably automated) tests to catch regressions for liveupdate in the tree view.
Flags: in-litmus?
Updated•17 years ago
|
Attachment #299225 -
Flags: approval1.9?
Updated•17 years ago
|
Attachment #299225 -
Flags: approval1.9? → approval1.9+
Updated•17 years ago
|
Keywords: checkin-needed
Comment 8•17 years ago
|
||
Checking in browser/components/places/content/treeView.js;
/cvsroot/mozilla/browser/components/places/content/treeView.js,v <-- treeView.js
new revision: 1.32; previous revision: 1.31
done
Status: REOPENED → RESOLVED
Closed: 17 years ago → 17 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Comment 9•17 years ago
|
||
this regressed drag & drop in the Library
now if you drag an item in the empty space under a tree it is inserted at the wrong index (one place before!)
STR:
- open in the library a bookmark folder
- take a bookmark and drag to bottom into the empty space
- the bookmark is inserted in the wrong place, should be appended at the end
Comment 10•17 years ago
|
||
reopening due to library D&D regression
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Comment 11•17 years ago
|
||
resetting as fixed (sorry) and opening a new one on the regression
Status: REOPENED → RESOLVED
Closed: 17 years ago → 17 years ago
Resolution: --- → FIXED
Updated•17 years ago
|
Flags: blocking-firefox3? → blocking-firefox3+
Comment 12•17 years ago
|
||
I can verify this bug with Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.4; en-US; rv:1.9b4pre) Gecko/2008021704 Minefield/3.0b4pre ID:2008021704 and the given steps in comment 3.
But there is one thing which looks curios. After step 6 I clicked again on http://winscp.net/ and now I get an extra entry under winscp.net with the same name. The title isn't used now. It looks like following:
winscp.net
WinSCP :: Free...
WinSCP :: Supported...
winscp.net
I think it should be filed as a new bug?
Comment 13•15 years ago
|
||
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.
Description
•