It appears that with bug 1150678 landed an URL can only one keyword. This causes two problems: -The obvious: one can't have the different hotkeys for the same URL even if the user creates multiple bookmarks. If I change a keyword it changes for all bookmarks with an identical url. This is in conflict with how the UI shows that every bookmark can have a keyword, which is not the case anymore -Keyworded searches - which is I believe the reason bookmark keywords exits can be different to the same URL. Some searches are not url coded and their data is stored differently(POST data, I assume). These searches are now destroyed. All of the pre-existing searches now show the same keyword. And results are not produced for the keyword. Only the bookmark's url is loaded. Also, all the hotkeys are still valid when typing in the location bar. But similarly neither search anymore. Just load the url. This also prevents me from creating new searches, for the same url without overwriting all the others' hotkeys.

I experienced other weirdnesses. I removed to be recreated, just to realize and can't. Then pressed ctrl+z to undo it. I kept pressing, to be sure I undid everything, but the item started to multiply. Also the keyword for all changed to one of the other old keywords.

(In reply to avada from comment #0) > It appears that with bug 1150678 landed an URL can only one keyword. > This causes two problems: > -The obvious: one can't have the different hotkeys for the same URL even if > the user creates multiple bookmarks. That's correct and by design. You can have multiple keywords to the same url only if the POST data differs. If I change a keyword it changes for > all bookmarks with an identical url. This is in conflict with how the UI > shows that every bookmark can have a keyword, which is not the case anymore Yes, the UI should be changed in bug 648398. > -Keyworded searches - which is I believe the reason bookmark keywords exits > can be different to the same URL. Some searches are not url coded and their > data is stored differently(POST data, I assume). These searches are now > destroyed. Sounds like bug 1310737? It just landed. So, this is either invalid for the first report, or a dupe for the second one. With the first nightly containing the fix for bug 1310737 you should be able to confirm if keywords using POST are working again.

(In reply to avada from comment #1) > I experienced other weirdnesses. I removed to be recreated, just to realize > and can't. Then pressed ctrl+z to undo it. I kept pressing, to be sure I > undid everything, but the item started to multiply. > Also the keyword for all changed to one of the other old keywords. This sounds like another different bug report. Please report only one issue per bug, otherwise it's impossible for us to track them. If you have steps to reproduce the undo problem, please report it apart.

(In reply to Marco Bonardo [::mak] from comment #2) > (In reply to avada from comment #0) > That's correct and by design. You can have multiple keywords to the same url > only if the POST data differs. Well then it's still bug, because this is not the case. As I said I have multiple searches which only differ in post data, yet they started to show the same keyword. It must be different because I reliably searched in different sections of the website with them. Also, when I try to create a totally different search from the pre-existing ones, the keyword is loaded into the add bookmark window. So, in short: differentiating by post data is broken. (In reply to Marco Bonardo [::mak] from comment #2) > Sounds like bug 1310737? It just landed. > > So, this is either invalid for the first report, or a dupe for the second > one. It can't be the same. To me url coded searches always worked. I never experienced the first part of that bug. As for the second, I can't really test because of the above.

(In reply to Marco Bonardo [::mak] from comment #3) > (In reply to avada from comment #1) > > I experienced other weirdnesses. I removed to be recreated, just to realize > > and can't. Then pressed ctrl+z to undo it. I kept pressing, to be sure I > > undid everything, but the item started to multiply. > > Also the keyword for all changed to one of the other old keywords. > > This sounds like another different bug report. Please report only one issue > per bug, otherwise it's impossible for us to track them. > If you have steps to reproduce the undo problem, please report it apart. Can't really be sure until this postdata bug is fixed. It's also likely caused by it, since this was happening with the same searches that differ only in post data.

Tried with todays nightly. They still show the same keyword, but now neither keyword does even load the url. I just remain on the present page as if nothing was done. I'm gonna try removing all of them an recreating. (Though I suspect the keywords that are not shown won't be erased)

Well, none of the hotkeys were removed... (I deleted the old ones, ran the places maintenance addon, then restarted the browser.) At recreating with the old hotkeys, at first it didn't allow me to use the chosen keyword. Then when I changed it to something else I never used. One of the other previously used hotkeys were used... After this I creating the new searches went normally. Except that the keyword changed for the second and third time, but not the fourth(last). Now all show what I added for the third. Whenever I try to change the hotkey for one of them, the save button only reacts to the second click and hillariously, the shown hotkey changes to one of the other ones, the one I added before this. And now half the the hotkeys perform a google search somehow, even though I have keyword.enabled, disabled. O_O The other half does nothing. It's a broken mess. I'm not sure if it's one bug or several.

Could you please provide STR (with strings) to create some of these non working keywords? I tested several and all of them worked for me. Do you see any error reported to the Browser Console when no action happens? Also did you try temporarily disabling add-ons? some of them overload our methods and break things.

To be clear, I need steps to reproduce the bug in a clean profile, so that I can reproduce it locally. If the problem is caused by a corrupt keywords table, it would be really hard for me to try to figure out the problem.

(In reply to Marco Bonardo [::mak] from comment #9) > To be clear, I need steps to reproduce the bug in a clean profile, so that I I'd say that's a fault in methodology. If you only care for something that's reproducible in a sterile environment (aka new profile) you ignore heaps of bugs. Particularly in a case like this when you inherit the places database. That said I'll try checking for errors and addons when I'll have more time. BTW can you share some place where I can test with multiple search settings and post data? I can't think of anything besides the one I have, which is borked. It's rather rarely used. > If the problem is caused by a corrupt keywords > table, it would be really hard for me to try to figure out the problem. I wouldn't be surprised if the landed changes mucked of stuff for good. Isn't there a way to validate, fix keyword stuff? That would be useful, especially if others could be affected by this or similar issues. (I have and ran the "Places Maintenance" addon, though I suspect that doesn't help with keywords.)

You misunderstood me, I don't only look at things that can be reproduced in a clean environment. The problem is that to reproduce your problem I would need your profile. So either you are fine sending your places.sqlite privately to me by mail, or I should find a way to rebuild it, and that's hard. A bug in a clean profile means I can reproduce it more easily. Places Maintenance removed orphaned keywords (that is keywords no more associated to a valid url). The only keywords API bug I know about at the moment is bug 1313188, that could likely cause problems. I don't know of specific pages to test this, we usually write automated tests for that, but I don't have enough data to create one.

(In reply to Marco Bonardo [::mak] from comment #11) > You misunderstood me, I don't only look at things that can be reproduced in > a clean environment. The problem is that to reproduce your problem I would > need your profile. So either you are fine sending your places.sqlite > privately to me by mail, or I should find a way to rebuild it, and that's > hard. > A bug in a clean profile means I can reproduce it more easily. > > Places Maintenance removed orphaned keywords (that is keywords no more > associated to a valid url). > > The only keywords API bug I know about at the moment is bug 1313188, that > could likely cause problems. It's likely you couldn't test it if I would send it, since it's on a private site. Maybe I'll try erasing everything but the problematic stuff and share it like that. > > I don't know of specific pages to test this, we usually write automated > tests for that, but I don't have enough data to create one. I only meant any public website that has performs a (configurable) search using postdata.

Hi, looks like we've found another problem with POST keywords in bug 1315514, so the fix in bug 1310737 is not complete. Re: a common POST search field, looks like duckduckgo is one.

(In reply to Marco Bonardo [::mak] from comment #13) > Hi, looks like we've found another problem with POST keywords in bug > 1315514, so the fix in bug 1310737 is not complete. I'll keep an eye out for that bug. Also, I sent a mail to you with a places file with everything but the broken searches removed. (They remained broken)

thanks for letting me know, it was marked as spam, recovered it!

So, you don't have orphan keywords, all of them are associated to valid urls, that means maintenance di its work. I don't see any "n" keyword. "nx" and "nf" are both pointing to the same url, but with different postData. This is unfortunately not manageable from the current bookmarking UI, it will need something like Bug 648398 or a keywords management add-on (I was evaluating creating one). That said, both of them should start working again once bug 1315514 lands. The other keywords I see are GET (since no postData) and should be working correctly already.

(In reply to Marco Bonardo [::mak] from comment #16) > So, you don't have orphan keywords, all of them are associated to valid > urls, that means maintenance di its work. > > I don't see any "n" keyword. > "nx" and "nf" are both pointing to the same url, but with different > postData. This is unfortunately not manageable from the current bookmarking > That said, both of them should start working again once bug 1315514 lands. > Hmm that only can be because they were overridden as I added them to different to the other postdata searches. This can only be, if the bookmark searches were not properly differentiated by post data. Is this covered by bug 1315514? > UI, it will need something like Bug 648398 or a keywords management add-on > (I was evaluating creating one). That'd be nice. At least I would know what's going on in the background instead of just guessing. BTW how will 648398 go? The ability for bookmarks to have a keyword are effectively removed now, they keywords are bindend to urls[+postdata], so they can't be showed in the bookmarks. If they're merged the searches will be spammed with non-search keyworded bookmarks. Or be separated into yet another section. Unless you split the keywords for searches and bookmarks. So that every bookmark could have it's own keyword but no searching ability. > The other keywords I see are GET (since no postData) and should be working > correctly already. There are more than one? Looking at what I sent one bookmark search somehow accidentally remained with the "fi" keyword. (and a keyworded bookmark which isn't a search)

(In reply to avada from comment #17) > Hmm that only can be because they were overridden as I added them to > different to the other postdata searches. This can only be, if the bookmark > searches were not properly differentiated by post data. Is this covered by > bug 1315514? no, that bug only makes existing POST keywords work in the locationbar. The bookmarking ui will keep being broken. > BTW how will 648398 go? The ability for bookmarks to have a keyword are > effectively removed now, they keywords are bindend to urls[+postdata], so > they can't be showed in the bookmarks. > If they're merged the searches will be spammed with non-search keyworded > bookmarks. Or be separated into yet another section. Right, The idea is that keywords you create through "Add Keyword For This Search" will become custom search engines, and as such they will be managed as search engines. Bookmark Keywords without a search string will be an advanced feature that we won't directly support in the Firefox UI. They will keep working, but you'll need a very simple add-on to manage them. This is still not carved in stone though. > > The other keywords I see are GET (since no postData) and should be working > > correctly already. > There are more than one? Looking at what I sent one bookmark search somehow > accidentally remained with the "fi" keyword. (and a keyworded bookmark which > isn't a search) Yes there are more than one, you can open places.sqlite with a Sqlite browser and check the moz_keywords table.

(In reply to Marco Bonardo [::mak] from comment #18) > no, that bug only makes existing POST keywords work in the locationbar. > The bookmarking ui will keep being broken. Well, then I think it's likely there are more bugs. > Yes there are more than one, you can open places.sqlite with a Sqlite > browser and check the moz_keywords table. This also sounds like a bug. Since they weren't removed with their associated bookmarks.

Still broken with latest nightly. Steps to reproduce in a new browser profile: 1. visit https://www.limetorrents.cc/ 2. right click on the search field and add a keyword 3. change category and then repeat an number of times. The keyword will be the last one added for all searches. And only the last search will work.

I see what you mean. The last keyword works though, so the location bar regression is fixed. The problem is that the bookmarks dialog can only handle one keyword at a time, and it just picks one randomly. there's no way to fix that bug with the current UI, we need something that can handle multiple keywords for the same url. That's bug 648398. We'll meetup in December to decide priorities, I'll bring that redesign as an opportunity.

I think we could workaround the problem in the bookmarking ui code (editBookmarkOverlay.js), when we detect the presence of a previous keyword, we could compare the postData and don't provide a previous keyword if postData differs. While this wouldn't allow to edit an existing keyword, it would allow to create multiple.

(In reply to Marco Bonardo [::mak] from comment #22) > I think we could workaround the problem in the bookmarking ui code > (editBookmarkOverlay.js), when we detect the presence of a previous keyword, > we could compare the postData and don't provide a previous keyword if > postData differs. That would be a definite improvement over the current impossibility, of having multiple searches. BTW aren't regressions like this are given a bit of priority? > > While this wouldn't allow to edit an existing keyword, it would allow to > create multiple. Why couldn't the UI for BM editing be similarly enhanced to check for postdata?

(In reply to avada from comment #23) > BTW aren't regressions like this are given a bit of priority? Keywords are a barely used Firefox feature, let alone POST keywords. When you think you may have time to fix it, something else with higher priority hits you. > > While this wouldn't allow to edit an existing keyword, it would allow to > > create multiple. > > Why couldn't the UI for BM editing be similarly enhanced to check for > postdata? A url can have multiple keywords due to POST data, but the current UI has only one keyword input field. The only way to workaround that would be to have it work like tags, so multiple keywords separated by commas. But since this ui is planned to be dismissed by bug 648398, it's unlikely anyone in Mozilla can spend time to add that multi-keyword support.

let's mutate this bug to track the add keyword ui workaround, the editing ui is still bug 648398.

Comment on attachment 8811310 [details] Bug 1314013 - Allow the bookmarking ui to add a new keyword when the provided postData differs from the existing one. https://reviewboard.mozilla.org/r/93462/#review93556 Thanks for the test. ::: browser/components/places/content/editBookmarkOverlay.js:144 (Diff revision 1) > if (!newKeyword) { > let entries = []; > yield PlacesUtils.keywords.fetch({ url: this._paneInfo.uri.spec }, > e => entries.push(e)); > if (entries.length > 0) { > - this._keyword = newKeyword = entries[0].keyword; > + // We show an existing keyword if: Could be clearer by saying "if either" or something like that. i.e., If either there's no post data, or, if there is post data, if the post data is the same.

Pushed by mak77@bonardo.net: https://hg.mozilla.org/integration/autoland/rev/946d7daf8984 Allow the bookmarking ui to add a new keyword when the provided postData differs from the existing one. r=adw

Okay. So I tried it. I added multiple keywords and they all work. As was mentioned editing is broken, all show the same keyword. So I don't think I can even change their title without ruining all of them.

Comment on attachment 8811310 [details] Bug 1314013 - Allow the bookmarking ui to add a new keyword when the provided postData differs from the existing one. Approval Request Comment [Feature/regressing bug #]: bug 1150678 [User impact if declined]: if a website has multiple search fields with different post data (for example multiple categories for the search), it's not possible to add more than one keyword through "Add keyword for this search" [Describe test coverage new/current, TreeHerder]: automated test [Risks and why]: Low risk, the code change only affects the keyword POST case [String/UUID change made/needed]: none

Comment on attachment 8811310 [details] Bug 1314013 - Allow the bookmarking ui to add a new keyword when the provided postData differs from the existing one. Fix an issue related to add keywords for bookmarks. Beta51+ and Aurora52+. Should be in 51 beta 2.