Chrome Version: 68
Firefox Version: 61 (I also tried 52)
OS: Linux (presumably all desktop)
What steps will reproduce the problem?
(1) Open Firefox.
(2) In Firefox, go to https://imdb.com.
(3) Right-click main search box in the IMDB page -> Add a Keyword for this Search.
(4) In the dialog, type "imdb" as the keyword.
(5) Close Firefox.
(6) In Chrome (recommend a throwaway profile!), go to Settings and click Import.
(7) Press Import.
What is the expected result?
The "imdb" keyword search is imported. Typing "imdb<space>query" into the Omnibox should search IMDB. IMDB should appear in chrome://settings/searchEngines.
What happens instead?
IMDB is imported as a *bookmark*. In the Bookmarks Bar, "Imported From Firefox" contains an IMDB bookmark, which leads to a 404 because it just has "%s" without any query.
I looked into it. It seems the Firefox sqlite database (places.sqlite) being exported by Firefox 52 and 61 (at least) does not correspond to their documentation (https://developer.mozilla.org/en-US/docs/Mozilla/Tech/Places/Database), which Chrome assumes is correct. The moz_bookmarks table is supposed to have a keyword_id column with a primary key to the moz_keywords table. However, I have never seen this column contain any value. Thus we see:
sqlite> SELECT id, title, keyword_id, fk FROM moz_bookmarks;
16|Search IMDb - Movies, TV and Celebrities - IMDb||88
The third column is empty; it should contain 1, the key to the corresponding row in moz_keywords:
sqlite> SELECT id, keyword, place_id FROM moz_keywords;
1|imdb|88
However, the moz_bookmarks.fk always corresponds to the moz_keywords.place_id. So we can join on this instead (and presumably, that's how Firefox matches the keyword with the bookmark):
sqlite> SELECT b.id, b.title, k.keyword FROM moz_bookmarks b LEFT JOIN moz_keywords k ON b.fk = k.place_id;
16|Search IMDb - Movies, TV and Celebrities - IMDb|imdb
🎉
This is a one-line fix in firefox_importer.cc:
- "LEFT JOIN moz_keywords k ON k.id = b.keyword_id "
+ "LEFT JOIN moz_keywords k ON b.fk = k.place_id "
There are *no tests for this* and it's very hard to write one. The test suite for firefox_importer consists of a single giant places.sqlite file dumped from Firefox one day, which has no keywords in it. It would be annoying to have to add a new test for this because you'd have to dump a new sqlite database from historical versions of Firefox.
TBH I'm a little terrified of taking this on. I'm 4 yak-shaves deep at the moment (Issue 849998, Issue 868214, Issue 868767) and while the code can be fixed easily, I'm not sure constructing a test case for this will be a good use of my time.