1541063 - Tags gets duplicated inside the Library when attempting to change the Location

Status: NEW

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

Description

[Anca Soncutean, Desktop QA]

Attachment: screencast issue.gif

[Affected versions]:

• 66.0.2
• 67.0b7
• 68.0a1 (2019-04-01)

[Affected platforms]:

• Win 10 x64
• Mac OS 10.10
• Ubuntu 18.04 x64

[Steps to reproduce]:

1. Bookmark any site via Star icon, given several tags (eg. a, b, c )
2. Go to Library (Ctrl+Shift+B)
3. Open the tags dropdown
4. Inside Location paste another link
5. Click one of the tag

[Expected result]:

• The respective tag is now unselected.

[Actual result]:

• Tags are duplicated.

[Regression range]:

• I will determine one asap.

[Additional Notes]:

• I can reproduce this intermittent, sometimes with other workarounds. I will investigate more on this matter for better conclusions.
• Collapsing the dropdown fixes the problem.

Comment 1

[Alice0775 White]

STR2:

1. Start Firefox with new profile
2. Click on Star Icon
3. Expand Tags list view
4. Type a,b in Tags input field (do not press enter key)
5. Click on Name input field

Regression window:
https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=b89b6c6da94b95abf74a79797f58542acb3e55c1&tochange=d60086281de9b5d9fe804e76c2c1721850b0f730

Regressed by:
d60086281de9 Marco Bonardo — Bug 1452067 - Move AllTags to an async method in the bookmarking API. r=Standard8

Comment 2

[Marco Bonardo [:mak]]

I'll have a look at this.

Comment 3

[Marco Bonardo [:mak]]

Anyway, I suspect the problem is that when you click on the tag in the selector, you blur the location field, and that causes the url change that notifies an onItemChanged that invokes the now async _rebuildTagsSelectorList. But at the same time you click on a tag, causing a racing tag change notification that races with the former one.

Comment 4

[Marco Bonardo [:mak]]

Attachment: Bug 1541063 - Tags get duplicated in the bookmarks dialog when the location changes at the same time. r=Standard8

Comment 5

[Mark Banner (:standard8)]

Marco and I have been discussing the patch and various issues that I'm still seeing. The issues seem to occur for me in this order:

• Changing the URL, then selecting a tag without a blur in-between.
• Mousedown invokes toggleItemCheckbox which starts to update the tags.
• This calls into _updateTags and then _setTagsFromTagsInputField
• _setTagsFromTagsInputField has an internal async function, setTags which is used to run the transactions to update the tags.
• At the point that function is queued (probably in the batch handling), the blur is able to kick in for the location change.
• onLocationFieldChange is called which updates the URL.
• Only then does setTags get executed, and at this point it is dealing with the old URL rather than the new one.

Comment 6

[Patricia Lawless]

hi mak, are you still working on this one?
thanks,
-Patricia

Comment 7

[Marco Bonardo [:mak]]

Yes, but higher priority stuff hit me and this ends up being a lot more complex than expected.

Comment 8

[neznajka97]

Attachment: Bug 1541063 - Tags get duplicated in the bookmarks dialog when the location changes at the same time. r=Standard8

Comment 9

[neznajka97]

Attachment: Bug 1541063 - Tags get duplicated in the bookmarks dialog when the location changes at the same time. r=Standard8

Comment 10

[Marco Bonardo [:mak]]

Attachment: Bug 1541063 - Tags gets duplicated inside the Library when attempting to change the Location. r=Standard8

Comment 11

[Chris Bellini (:cbellini)]

Verify is this has been resolved by recent updates.

Comment 12

[Marco Bonardo [:mak]]

Most of the steps work correctly, what I can still reproduce:

1. open the tags selector
2. put a,b,c in the tags field and click on Location field
3. edit the url, for example by adding a 2 at the end
4. click on the first tag (it goes away)
5. click again on the first tag

At this point all the tags are unchecked, even if "c" is still on that url

I think this is a bit less critical than the original issue, even if we should definitely investigate and fix it.