Bookmark keywords cannot be used or updated if a keyword points to an invalid url

RESOLVED FIXED in Firefox 55

Status

()

defect
P2
normal
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: zebediah49, Assigned: mak)

Tracking

52 Branch
Firefox 55
Points:
---
Bug Flags:
qe-verify +

Firefox Tracking Flags

(firefox55 fixed)

Details

(Whiteboard: [fxsearch])

Attachments

(1 attachment)

(Reporter)

Description

2 years ago
User Agent: Mozilla/5.0 (X11; Ubuntu; Linux x86_64; rv:52.0) Gecko/20100101 Firefox/52.0
Build ID: 20170303013352

Steps to reproduce:

Updated to Firefox 52.0 (64-bit) (Ubuntu 12.04 edition)


Actual results:

I attempted to use a keyword-triggered quicksearch, and got a regular search for the keyword.  In other words, typing "g firefox" to search for firefox with the keyword 'g' forwarding to google, I instead was brought to the default search page, searching for the entire string "g firefox".

I went into bookmarks to investigate, and found that all of my quick-searches had no keyword any more.

Furthermore, attempting to enter a value for the keyword did not help.  Instead, every single bookmark shows the same value for the keyword field (as if the text box isn't even connected to anything).

Attempting to restore my bookmarks via the 'import and backup' dialog to a backup from today simply returns "Unable to process backup file".


Expected results:

1. Quick searches should quick-search.
2. Setting a keyword for a quick-search should actually set the keyword.
2a. Setting a keyword for one bookmark should only affect that bookmark's keyword
3. Restoring the bookmark settings should restore them.

Updated

2 years ago
Component: Untriaged → Bookmarks & History
Try cleaning up your db with this add-on, then try again adding your keyword
https://addons.mozilla.org/en-US/firefox/addon/places-maintenance/
(Reporter)

Comment 2

2 years ago
(In reply to Marco Bonardo [::mak] from comment #1)
> Try cleaning up your db with this add-on, then try again adding your keyword
> https://addons.mozilla.org/en-US/firefox/addon/places-maintenance/

No problems found; as far as I can tell it didn't change much (final DB size == initial size).  Keyword problem persists.
(Reporter)

Comment 3

2 years ago
I have pinpointed and resolved the issue, at least for myself.  

I noticed that suggestions of history also wasn't working, so I tried running firefox with a new profile, which did not have the issue.  In the process, running as my user, I got a significant number of errors to console, all of which said

A coding exception was thrown and uncaught in a Task.

    Full message: TypeError: http://document.body.innerhtml=document.body.innerhtml.replace("img/mask_v2%22,%20%22photo/%22+document.URL.split(%27/%27)%5B4%5D+%22_700b%22); is not a valid URL.
    Full stack: @resource://gre/modules/PlacesUtils.jsm:2333:28
    TaskImpl_run@resource://gre/modules/Task.jsm:319:42
    Handler.prototype.process@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:932:23
    this.PromiseWalker.walkerLoop@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:813:7
    this.PromiseWalker.scheduleWalkerLoop/<@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:747:11

Following this, PlacesUtils.jsm:2333 calls the SQL query `SELECT keyword, url, post_data FROM moz_keywords k JOIN moz_places h ON h.id = k.place_id;`, which I replicated myself, and found that there was a keyword (it must have been there from ages ago, because I don't remember it) with the "url" ` http://document.body.innerhtml=document.body.innerhtml.replace("img/mask_v2%22,%20%22photo/%22+document.URL.split(%27/%27)%5B4%5D+%22_700b%22);`.  Exiting firefox, deleting the keyword for it directly from the SQL (`DELETE FROM moz_keywords WHERE keyword='<keyword>'`), and restarting firefox returned it to usable condition.

It would appear that a change introduced in 52.0 causes keyword-triggered bookmarlets to destroy the entire functionality of the keyword system.
(In reply to zebediah49 from comment #3)
>     Full message: TypeError:
> http://document.body.innerhtml=document.body.innerhtml.replace("img/
> mask_v2%22,%20%22photo/%22+document.URL.split(%27/%27)%5B4%5D+%22_700b%22);
> is not a valid URL.

> It would appear that a change introduced in 52.0 causes keyword-triggered
> bookmarlets to destroy the entire functionality of the keyword system.

The problem is different, that url is invalid and any code trying to resolve it would throw. Other features could have been broken due to it.
Now, what I can't explain is that all of our APIs pass any url through sanitization, so it's impossible for Firefox itself to add that url into the database. The only way would be to directly insert it through a raw SQL query or editing in a Database Manager of some sort.

Maybe it was added by an add-on?
Have you ever edited the database manually through SQL in the past?

It could be worth, on our side, to add a task to db maintenance to test all the urls and remove invalid ones.
and we could also add a try/catch around keywords fetching to eventually paper over an invalid url.
Status: UNCONFIRMED → NEW
Ever confirmed: true
Priority: -- → P2
Summary: Bookmark keywords cannot be used or updated → Bookmark keywords cannot be used or updated if a keyword points to an invalid url
Assignee: nobody → mak77
Whiteboard: [fxsearch]

Comment 7

2 years ago
mozreview-review
Comment on attachment 8867263 [details]
Bug 1345349 - Places keywords break if one of the keywords points to an invalid url

https://reviewboard.mozilla.org/r/138802/#review142464

Looks good. r=Standard8
Attachment #8867263 - Flags: review?(standard8) → review+

Comment 8

2 years ago
Pushed by mak77@bonardo.net:
https://hg.mozilla.org/integration/autoland/rev/4500a7b7bbd5
Places keywords break if one of the keywords points to an invalid url r=standard8
Status: NEW → ASSIGNED

Comment 9

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/4500a7b7bbd5
Status: ASSIGNED → RESOLVED
Last Resolved: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 55
Flags: qe-verify+
You need to log in before you can comment on or make changes to this bug.