1313620 - Urlbar add-ons "URL Alias" and "Keyword Search" no longer work when performing search in the location bar

Status: RESOLVED WONTFIX

(Firefox :: Address Bar, defect)

Description

[Torvin]

User Agent: Mozilla/5.0 (Windows NT 6.3; Win64; x64; rv:51.0) Gecko/20100101 Firefox/51.0
Build ID: 20161015004013

Steps to reproduce:

Two of addons I have no longer work in the latest Firefox Developer Edition version: URLAlias (https://addons.mozilla.org/en-US/firefox/addon/url-alias-8703/) and Keyword Search (https://addons.mozilla.org/en-US/firefox/addon/keyword-search/). I use them daily and it's very inconvenient.
My current version (update issued on 15 Oct) is not affected yet, but the latest version definitely is. So something happened in between those two. I see no error messages in the browser console. 


Actual results:

Aliases from URLAlias are now ignored, entered text goes straight to the search engine. "Browse by Name" feature from Keword Search no longer works.


Expected results:

Addons work as expected

Comment 1

[Loic]

I tested both add-ons and they are affected by the same regression:
https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=64cd7d87e78cf2e4b919aad10fb9f4962834b004&tochange=70d19ed29c980ba3d6825e5286bda08fa7dc676f

Marco Bonardo — Bug 1306639 - Searching in locationbar by typing something and pressing enter is not accounted in telemetry. r=adw

Comment 2

[Marco Bonardo [:mak]]

Does this differ from bug 1310737? if they are broken for Firefox itself, I don't see how they could be working in add-ons...

That said I don't know how those add-ons work, and I can't predict what will happen after we fix bug 1310737, but afaict, we never tracked add-ons not working, unless they had a very large population. They may just go back working or need some fixed on their side.

Comment 3

[Mike Kaply [:mkaply]]

Keyword Search is my add-on, so I can help debug.

The main thing I do is listen to the keyword-search observer. Does that still work?

Comment 4

[Mike Kaply [:mkaply]]

Actually, no, it's something different.

I have a hack in my code to detect keywords. If the aPurpose on the submission is "keyword" and the response type is null, it's a keyword.

Your patch changed response type to be null in non keyword cases. So it might be keyword related.

Comment 5

[Marco Bonardo [:mak]]

keyword-search is only invoked when something is "fixed" by the docshell and was added for telemetry purposes. The current awesomebar backend already fixes things by itself, and telemetry can now use the moz-action parameters to track data more accurately, so in many cases "keyword-search" won't be invoked anymore.
So that could still be related.

Comment 6

[Mike Kaply [:mkaply]]

So I've worked around the Keyword Search problem and uploaded a new version.

My problem was that I was relying on a certain behavior in getShortcutOrURIAndPostData to detect search keywords.

That function is no longer called.

I've made my code more robust in that I actually look at the value of the URL bar and attempt to determine if it was a search keyword.

Comment 7

[Torvin]

(In reply to Mike Kaply [:mkaply] from comment #6)
> I've made my code more robust in that I actually look at the value of the
> URL bar and attempt to determine if it was a search keyword.

@mkaply is this new detection code compatible with addons like URL Alias?

Comment 8

[Mike Kaply [:mkaply]]

> @mkaply is this new detection code compatible with addons like URL Alias?

I don't know what exactly broke in URL Alias. I'll contact the author

Comment 9

[Marco Bonardo [:mak]]

I plan to reintroduce a fallback for getShortcutOrURI in bug 1310737 for add-ons compat. I don't know if it's going to fix URL Alias, we can verify that once we have a fixed Nightly.

Comment 10

[zibada]

URL Alias author here.

I simply override window.getShortcutOrURIAndPostData (or window.getShortcutOrURI if window.getShortcutOrURIAndPostData is undefined) with my own wrapper and do all rewriting in it.

Looks like window.getShortcutOrURIAndPostData is no longer called at all, although it is still defined.

Would be glad to hear any quick fix suggestions.

Comment 11

[Marco Bonardo [:mak]]

(In reply to zibada from comment #10)
> URL Alias author here.

Hi, it looks like most of your add-on functionality is already covered by normally defining a keyword through PlacesUtils.keywords API... could you please point out what's left?
Off-hand looks like these 2 features are not covered:

* more complicated substitions: you can specify %s in replacement URL to insert query text to its place instead of appending.
Example: "lj http://%s.livejournal.com/" will transform "lj someuser" to http://someuser.livejournal.com/

(we only allow to replace %s in params, not in the domain or path)

* [new in 2.2.0] you can specify %1, %2, ... in replacement URL to insert part of query text to its place.
Any leftover tokens are appended to query string as per above rules.
Example: "w http://%1.wikipedia.org/wiki/" will transform "w ru Firefox" to http://ru.wikipedia.org/wiki/Firefox

(we don't allow more than one replacement)

I will think about your case and see if there's something simple I can suggest or add as a replacement.

Comment 12

[zibada]

(In reply to Marco Bonardo [::mak] from comment #11)
> Hi, it looks like most of your add-on functionality is already covered by
> normally defining a keyword through PlacesUtils.keywords API... could you
> please point out what's left?
> Off-hand looks like these 2 features are not covered:

Yes, it's quite similar to built-in keyword search, in fact, I created this extension mostly because UI for keyword management is VERY non-intuitive.

While it could be rewritten as an alternate UI for keyword API, the current approach is both simpler and more flexible. I personally use a couple of aliases with %s in domain/path parts, even considered adding regex match/replace at some point. (could be useful with livejournal handles to replace underscores with dashes, though they later fixed it on their side)

 
> I will think about your case and see if there's something simple I can
> suggest or add as a replacement.

It would be nice if this could be fixed with some minor changes on my side, like observing some event instead of redefining built-in functions, rather than complete rewrite.

Comment 13

[Marco Bonardo [:mak]]

considering the number of alias will be small, if you could keep them in memory, and thus answer to requests synchronously, you could just listen for Enter on the urlbar input field, and set input.popup.overrideValue. if needed that should allow to override anything the urlbar wants to confirm.

Comment 14

[Torvin]

(In reply to Mike Kaply [:mkaply] from comment #6)
> So I've worked around the Keyword Search problem and uploaded a new version.

Can confirm that the latest addon version now works. Thank you very much!

Comment 15

[zibada]

(In reply to Marco Bonardo [::mak] from comment #13)
> considering the number of alias will be small, if you could keep them in
> memory, and thus answer to requests synchronously, you could just listen for
> Enter on the urlbar input field, and set input.popup.overrideValue. if
> needed that should allow to override anything the urlbar wants to confirm.

Good idea, thanks.
Uploaded new version here:
https://addons.mozilla.org/ru/firefox/addon/url-alias-8703/versions/2.3.3

Seems to work on 51.0a2, though I have not tested on any previous versions.

Comment 16

[Torvin]

(In reply to zibada from comment #15)
> Seems to work on 51.0a2, though I have not tested on any previous versions.

Can confirm that both extensions work in the latest Dev version. Thank you very much!

Comment 17

[Gerry Chang [:gchang]]

Un-track for 51 as the add-on is fixed and verified.

Comment 18

[Marco Bonardo [:mak]]

So, looks like we have possible workarounds for addons and both cases here found a new valid hook.
Plus we have incoming chrome.omnibox webext API, and Drew is working on a new API to allow direct add-ons integration in the awesomebar results.
For now I'd mark this as a wontfix, since we can't make things working exactly as before. If other extensions break we can discuss possible hooks with them, but I'd suggest to use firefox-dev mailing list, or to open a separate report.

Comment 19

[Marcia Knous [:marcia]]

Tracking 52- based on resolution of won't fix.

Comment 20

[Torvin]

(In reply to zibada from comment #15)
Hey zibada, looks like URL Alias is still broken. steps to reproduce:

1) open any tab. e.g. http://google.com
2) open another tab next to it and use any alias in the url bar - URL Alias correctly transforms the url
3) switch back to the first tab and without changing anything in the url press enter in the url bar

expected result: tab reloads
actual result: tab navigates to the same address the second tab has

FF 52.0a2 64-bit, Windows 8.1

Comment 21

[Torvin]

(In reply to Torvin from comment #20)
Keyword Search doesn't have this problem

Comment 22

[zibada]

(In reply to Torvin from comment #20)
> (In reply to zibada from comment #15)
> Hey zibada, looks like URL Alias is still broken. steps to reproduce:
> 

Thanks for testing.
This happens because popup.overrideValue for urlbar doesn't get reset to null if Enter was pressed with no changes to URL, so it reuses the last assigned value.

Uploaded version 2.3.3.1, where I'm setting it manually to null now if no alias was found. Not entirely sure this is the right thing to do, as it could conflict with other extensions using the same mechanism.

Marco, could you clarify, is this the intended behavior or a bug?

Comment 23

[Marco Bonardo [:mak]]

(In reply to zibada from comment #22)
> Marco, could you clarify, is this the intended behavior or a bug?

Ah, onSearchBegin is not invoked in such a case. That means we need a different hook point to nullify the overrideValue, it may not be trivial.
Please file a separate bug and we'll look into that.

Comment 24

[Torvin]

@zibada, the fix works, thanks a lot!

@Marko, are you asking me to file a bug or zibada? Because I'm unaware of urlbar's internals and I'm not quite sure what you are talking about.

Comment 25

[Gerry Chang [:gchang]]

Mark 51 won't fix as no fix in our side.

Comment 26

[Marco Bonardo [:mak]]

I filed bug 1323682.

Comment 27

[Torvin]

(In reply to Marco Bonardo [::mak] from comment #26)
> I filed bug 1323682.

Thanks, Marco! Will Zibada need to remove his latest work-around once that is fixed?

Comment 28

[Marco Bonardo [:mak]]

Surely he could.