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

Status: VERIFIED FIXED

(Toolkit :: Places, defect, P2)

Description

[Virtual_ManPL [:Virtual] 🇵🇱 - (please needinfo? me - so I will see your comment/reply/question/etc.)]

[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

Comment 1

[Virtual_ManPL [:Virtual] 🇵🇱 - (please needinfo? me - so I will see your comment/reply/question/etc.)]

I suspect that it could be caused by bug #1360872.

Comment 2

[Virtual_ManPL [:Virtual] 🇵🇱 - (please needinfo? me - so I will see your comment/reply/question/etc.)]

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

Comment 3

[Virtual_ManPL [:Virtual] 🇵🇱 - (please needinfo? me - so I will see your comment/reply/question/etc.)]

and on Firefox restart bookmark name will reappear again

Comment 4

[Lina Butler [:lina]]

Attachment: Bug 1379412 - Don't fall back to the history visit title when fetching folder children.

Review commit: https://reviewboard.mozilla.org/r/155782/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/155782/

Comment 5

[Lina Butler [:lina]]

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.

Comment 6

[Marco Bonardo [:mak] (Away Apr 25 - May 5)]

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.

Comment 7

[Lina Butler [:lina]]

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

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/155782/diff/1-2/

Comment 8

[Pulsebot]

Pushed by [e-mail]:
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

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/e39010c3f233

Comment 10

[Virtual_ManPL [:Virtual] 🇵🇱 - (please needinfo? me - so I will see your comment/reply/question/etc.)]

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.

Comment 11

[Gerry Chang [:gchang]]

As this was fixed, no need to track for 56.