Closed
Bug 1437250
Opened 7 years ago
Closed 7 years ago
uncaught exception: Unexpected node when opening Bookmarks in Library
Categories
(Firefox :: Bookmarks & History, defect, P1)
Firefox
Bookmarks & History
Tracking
()
RESOLVED
FIXED
Firefox 60
Tracking | Status | |
---|---|---|
firefox-esr52 | --- | unaffected |
firefox58 | --- | unaffected |
firefox59 | --- | unaffected |
firefox60 | --- | fixed |
People
(Reporter: magicp.jp, Assigned: standard8)
References
Details
(Keywords: regression)
Attachments
(2 files)
Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:60.0) Gecko/20100101 Firefox/60.0 ID:20180209220324
Steps to reproduce:
1. Launch Nightly
2. Open Library button > Bookmarks
Actual results:
The following errors are logged in the browser console.
uncaught exception: Unexpected node browserPlacesViews.js:91:7
uncaught exception: Unexpected node PanelMultiView.jsm:128:5
Reproducible situation:
There is no recently bookmarked item, but it is different with "(empty)". So this cannot reproduce with a newer profile. Please find the attached image.
Expected results:
No errors.
Regression range:
https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=12de5643ae0ae6ad6a97797f2a07fe856f9b66e1&tochange=c388570c330fd7745f12eca3258cc3ae4b3d5bb2
Blocks: 1423896
Has Regression Range: --- → yes
Has STR: --- → yes
status-firefox58:
--- → unaffected
status-firefox59:
--- → unaffected
status-firefox-esr52:
--- → unaffected
Comment 1•7 years ago
|
||
Mark, do you have any time to take a look at this?
It sounds like a failure in _createDOMNodeForPlacesNode. Could be the nodes we autogenerate are somehow missing the type?
Flags: needinfo?(standard8)
Updated•7 years ago
|
Keywords: regression
Priority: -- → P1
Assignee | ||
Updated•7 years ago
|
Assignee: nobody → standard8
Flags: needinfo?(standard8)
Assignee | ||
Comment 2•7 years ago
|
||
Initial findings:
- As comment 0 says, this doesn't happen on newer profiles.
- The query being run is: place:queryType=1&sort=12&maxResults=42&excludeQueries=1
- On one of my dev profiles, this hits:
https://searchfox.org/mozilla-central/rev/b9f1a4ecba48b2d8c686669e32d109c40e927b48/browser/components/places/content/browserPlacesViews.js#2212
The type is a folder shortcut (9) which is not one of the root folders, but points to the toolbar folder, created during my testing when writing the initial patch.
- On another dev profile, the menu worked just fine. However, I can reproduce then this by dragging & dropping e.g. "Other Bookmarks" folder to a different sub-folder.
So I think there's two issues here:
1) How to handle folder shortcuts in the places panel (we probably shouldn't be displaying them).
2) The drag and drop is now creating folder shortcuts. I thought I'd fixed that before landing the patch, but maybe not.
Comment 3•7 years ago
|
||
(In reply to Mark Banner (:standard8) from comment #2)
> 1) How to handle folder shortcuts in the places panel (we probably shouldn't
> be displaying them).
What is the Places Panel here? A folder shortcut is pretty much a folder, its contents are writable, why should we hide it?
> 2) The drag and drop is now creating folder shortcuts. I thought I'd fixed
> that before landing the patch, but maybe not.
So currently copying a folder shortcut creates a folder shortcut. That could also be ok, on the other side copying a folder creates a copy of its contents, and shortcuts are unrecognizeable from folders in the UI, so the whole thing is a bit confusing in general... For now we should probably go back to the status-quo that was making a copy of the contents, as you suggest.
Assignee | ||
Comment 4•7 years ago
|
||
(In reply to Marco Bonardo [::mak] from comment #3)
> (In reply to Mark Banner (:standard8) from comment #2)
> > 1) How to handle folder shortcuts in the places panel (we probably shouldn't
> > be displaying them).
>
> What is the Places Panel here? A folder shortcut is pretty much a folder,
> its contents are writable, why should we hide it?
Sorry, I was referring to the "PlacesPanelview" code which seems to drive the "Recently Bookmarked" list from the "View history, saved bookmarks & more" button on the Toolbar (and then click Bookmarks).
I'd have thought we wouldn't want folder shortcuts/folders to be displayed there.
Comment 5•7 years ago
|
||
(In reply to Mark Banner (:standard8) from comment #4)
> I'd have thought we wouldn't want folder shortcuts/folders to be displayed
> there.
Yes, that menu should NOT show folders, it's a plain list of recent bookmarks.
OT: There is actually another bug in that menu, that is bookmarks dragged from it are being copied instead of moved, that is a bug in general of any query returning bookmarks. The original reason for the choice of queries copying contents is that when you search in the library you don't know where the bookmark comes from, thus it's hard to "organize things" when you don't know the origin. On the other side, this causes the user to create copies and copies of the same bookmarks around. FWIW, that is bug 1410896.
Comment hidden (mozreview-request) |
Comment 7•7 years ago
|
||
mozreview-review |
Comment on attachment 8952710 [details]
Bug 1437250 - Bookmark url queries shouldn't return folder shortcuts.
https://reviewboard.mozilla.org/r/221946/#review227828
Attachment #8952710 -
Flags: review?(mak77) → review+
Pushed by mbanner@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/f0668098ade0
Bookmark url queries shouldn't return folder shortcuts. r=mak
Comment 9•7 years ago
|
||
bugherder |
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 60
You need to log in
before you can comment on or make changes to this bug.
Description
•