1440644 - The New Bookmark dialog/window doesn't save the keyword

Status: VERIFIED FIXED

(Firefox :: Bookmarks & History, defect, P1)

Description

[Benjamin Dauth]

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.13; rv:58.0) Gecko/20100101 Firefox/58.0
Build ID: 20180206200532

Steps to reproduce:

open bookmarks ("Lesezeichen verwalten", cmd shift B)
right-click (actually ctrl+click)
new bookmark (Neues Lesezeichen)
enter name, address (copied), keyword
click add (Hinzufügen)


Actual results:

New bookmark got saved without the keyword
keyword search brought the usual google search instead of the intended one
I could then add a keyword just fine



Expected results:

Bookmark should contain keyword on first try
keyword should have worked in address bar

Comment 1

[Drew Willcoxon :adw]

Could you please go to about:support, scroll down to the Places Database section, click the Verify Integrity button, and report the output here in the bug?

Comment 2

[Benjamin Dauth]

> Task: checkIntegrity
+ The database is sane
> Task: invalidateCaches
+ The caches have been invalidated
> Task: checkCoherence
+ The database is coherent
> Task: expire
+ Database cleaned up
> Task: vacuum
+ Initial database size is 5120 KiB
+ The database has been vacuumed
+ Final database size is 5120 KiB
> Task: stats
+ Database size is 5120 KiB
+ pragma_user_version is 41
+ pragma_page_size is 32768
+ pragma_cache_size is -2048
+ pragma_journal_mode is wal
+ pragma_synchronous is 1
+ History can store a maximum of 69773 unique pages
+ Table moz_places has 1839 records
+ Table moz_historyvisits has 2578 records
+ Table moz_inputhistory has 18 records
+ Table moz_hosts has 184 records
+ Table moz_bookmarks has 41 records
+ Table moz_bookmarks_deleted has 0 records
+ Table moz_keywords has 18 records
+ Table sqlite_sequence has 1 records
+ Table moz_anno_attributes has 8 records
+ Table moz_annos has 14 records
+ Table moz_items_annos has 20 records
+ Table sqlite_stat1 has 18 records
+ Index sqlite_autoindex_moz_inputhistory_1
+ Index sqlite_autoindex_moz_hosts_1
+ Index sqlite_autoindex_moz_bookmarks_deleted_1
+ Index sqlite_autoindex_moz_keywords_1
+ Index sqlite_autoindex_moz_anno_attributes_1
+ Index moz_places_url_hashindex
+ Index moz_places_hostindex
+ Index moz_places_visitcount
+ Index moz_places_frecencyindex
+ Index moz_places_lastvisitdateindex
+ Index moz_places_guid_uniqueindex
+ Index moz_historyvisits_placedateindex
+ Index moz_historyvisits_fromindex
+ Index moz_historyvisits_dateindex
+ Index moz_bookmarks_itemindex
+ Index moz_bookmarks_parentindex
+ Index moz_bookmarks_itemlastmodifiedindex
+ Index moz_bookmarks_dateaddedindex
+ Index moz_bookmarks_guid_uniqueindex
+ Index moz_keywords_placepostdata_uniqueindex
+ Index moz_annos_placeattributeindex
+ Index moz_items_annos_itemattributeindex
> Task: _refreshUI

Comment 3

[Drew Willcoxon :adw]

Hmm, that all looks fine.  Would you please mind reproducing the problem in safe mode with add-ons disabled?  https://support.mozilla.org/en-US/kb/troubleshoot-firefox-issues-using-safe-mode

Comment 4

[Benjamin Dauth]

Created fresh profile (about:profiles), started profile same deal, Menu->Help->Restart with Add-ons Disabled Entering safe mode
Still no difference

Comment 5

[Drew Willcoxon :adw]

OK, I'm seeing this too, on Nightly.  The New Bookmark dialog seems to just not save the keyword.

Comment 6

[Marco Bonardo [:mak]]

I'll look into this in the next days.

Just to be sure, it's possible you are trying to reuse an already existing keyword, or to assign a different keyword to an url that already has one?

Comment 7

[Marco Bonardo [:mak]]

hm, Services.ui is undefined, looks like a typo I introduced

Comment 8

[Drew Willcoxon :adw]

I see that error and this one too:

> Error: Invalid value for input property postData: Error: Invalid value
> resource://gre/modules/PlacesTransactions.jsm:778:17 validateValue
> resource://gre/modules/PlacesTransactions.jsm:785:16 validateInput
> resource://gre/modules/PlacesTransactions.jsm:850:10 DefineTransaction.validatePropertyValue
> resource://gre/modules/PlacesTransactions.jsm:897:24 DefineTransaction.verifyInput
> resource://gre/modules/PlacesTransactions.jsm:696:15 ctor
> resource://gre/modules/PlacesTransactions.jsm:692:14 ctor
> chrome://browser/content/places/editBookmarkOverlay.js:672:5 onKeywordFieldChange
> chrome://browser/content/places/bookmarkProperties.xul:1:1 onchange
> resource:///modules/PlacesUIUtils.jsm:294:5 showBookmarkDialog
> chrome://browser/content/places/controller.js:714:7 newItem
> self-hosted:1211:9 next

Comment 9

[Marco Bonardo [:mak]]

Actually the typo is just from bug 1439315, I'm not sure about the other blockers? Are those about comment 8?

Comment 10

[Marco Bonardo [:mak]]

Attachment: Bug 1440644 - Fix a typo preventing the bookmark dialog storing a keyword.

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

Comment 11

[Mark Banner (:standard8)]

Comment on attachment 8956401 [details]
Bug 1440644 - Fix a typo preventing the bookmark dialog storing a keyword.

https://reviewboard.mozilla.org/r/225288/#review231282

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/dc1a57e1430e
Fix a typo preventing the bookmark dialog storing a keyword. r=standard8

Comment 13

[Alice0775 White]

(In reply to Marco Bonardo [::mak] from comment #9)
> I'm not sure about the other
> blockers? Are those about comment 8?

No, about the behavior of bug title in m-c and beta58 channel.

Comment 14

[Marco Bonardo [:mak]]

It seems to save the keyword if I edit the bookmark, not on creation. We may uplift a partial patch (we don't need to fix the typo on 59).

Comment 15

[Dorel Luca [:dluca]]

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

Comment 16

[Marco Bonardo [:mak]]

considered the timing (late in the beta cycle) and the affected population (very few use keywords and this is restrict to creating a bookmark from scratch), I'm going to call this a wontfix on 59. It doesn't seem to have the importance to candidate for late 59.

Comment 17

[David Olah]

I verified the issue and based on Comment 16 it could be marked as verified, but the issue is not 100% fixed.

I tried on Nightly 60.0a1 (2018-03-11) (64-bit) Mac OS 10 and Windows 10.0 x64 and the keyword is saved but uppercase characters are always saved as lowercase.

Marco, should I reopen this issue or create another one for it and mark this one as verified?

Comment 18

[Marco Bonardo [:mak]]

(In reply to David Olah from comment #17)
> I tried on Nightly 60.0a1 (2018-03-11) (64-bit) Mac OS 10 and Windows 10.0
> x64 and the keyword is saved but uppercase characters are always saved as
> lowercase.
> 
> Marco, should I reopen this issue or create another one for it and mark this
> one as verified?

Keywords are forced lowercase by design, so I don't think that's a bug?

Comment 19

[David Olah]

Based on Comment 18 and Bug 1087459 I am closing the issue as verified