Bookmark created with blank/no name still has bookmark name in some cases and on browser restart after landing patch from bug #1360872

VERIFIED FIXED in Firefox 56

Status

()

Toolkit
Places
P2
major
VERIFIED FIXED
4 months ago
4 months ago

People

(Reporter: Virtual, Assigned: kitcambridge)

Tracking

({nightly-community, regression, reproducible})

56 Branch
mozilla56
x86_64
Windows 7
nightly-community, regression, reproducible
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox-esr52 unaffected, firefox54 unaffected, firefox55 unaffected, firefox56 verified)

Details

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(1 attachment)

[Tracking Requested - why for this release]: Regression


STR:
1. Add some bookmark with blank name to bookmarks
and see that added bookmark have name,
instead of having blank name with only favicon

Workaround: Clear name second time to get blank name
I suspect that it could be caused by bug #1360872.
Blocks: 1360872
Flags: needinfo?(Virtual)
Better reproducible STR:
1. Bookmark this website page ( http://www.ldoceonline.com/ ) with blank/no name in "Bookmark Tooolbar" folder
2. Open Bookmark doorhanger and see that bookmark doesn't have name
3. Open Library and see that bookmark still have name
Component: Session Restore → Places
Flags: needinfo?(Virtual)
Keywords: reproducible
Product: Firefox → Toolkit
Has STR: --- → yes
Summary: Bookmark created with blank/no name have bookmark name → Bookmark created with blank/no name still have bookmark name in some cases
Summary: Bookmark created with blank/no name still have bookmark name in some cases → Bookmark created with blank/no name still has bookmark name in some cases
and on Firefox restart bookmark name will reappear again
Flags: needinfo?(kit)
Summary: Bookmark created with blank/no name still has bookmark name in some cases → Bookmark created with blank/no name still has bookmark name in some cases and on browser restart
Keywords: nightly-community

Updated

4 months ago
Priority: -- → P2
Comment hidden (mozreview-request)
(Assignee)

Comment 5

4 months ago
I think this worked before because we stored blank titles as `""`, so `IFNULL(b.title, h.title)` would return the empty string. Now, we store blank titles as `NULL`, so `IFNULL` would fall back to the visit title. This patch fixes the toolbar and Library views, but I'm wondering if it has other unintended consequences.
Flags: needinfo?(kit)
Has Regression Range: --- → yes
Keywords: regressionwindow-wanted
Summary: Bookmark created with blank/no name still has bookmark name in some cases and on browser restart → Bookmark created with blank/no name still has bookmark name in some cases and on browser restart after landing patches from bug #1360872
Assignee: nobody → kit
Status: NEW → ASSIGNED
Summary: Bookmark created with blank/no name still has bookmark name in some cases and on browser restart after landing patches from bug #1360872 → Bookmark created with blank/no name still has bookmark name in some cases and on browser restart after landing patch from bug #1360872

Comment 6

4 months ago
mozreview-review
Comment on attachment 8884941 [details]
Bug 1379412 - Don't fall back to the history visit title when fetching folder children.

https://reviewboard.mozilla.org/r/155782/#review161614

Can we add a simple xpcshell-test for this? It may be just matter of checking titles in an nsNavHistoryQuery result after adding a bookmark with an empty title.
Attachment #8884941 - Flags: review?(mak77) → review+
Comment hidden (mozreview-request)

Comment 8

4 months ago
Pushed by kcambridge@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/e39010c3f233
Don't fall back to the history visit title when fetching folder children. r=mak

Comment 9

4 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/e39010c3f233
Status: ASSIGNED → RESOLVED
Last Resolved: 4 months ago
status-firefox56: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla56
Blocks: 1380740
I'm confirming that it's fixed, starting in Mozilla Firefox 56.0a1 (2017-07-13) (64-bit).
It's fixed partially, so leftover will be tracked in new bug #1380740.
Thanks. I'm marking this bug as VERIFIED.
Status: RESOLVED → VERIFIED
status-firefox56: fixed → verified
As this was fixed, no need to track for 56.
tracking-firefox56: ? → -
tracking-firefox56: - → ---
QA Contact: Virtual
You need to log in before you can comment on or make changes to this bug.