Only one website in the SiteDataManager list can be selected

VERIFIED FIXED in Firefox 60

Status

()

defect
P1
normal
VERIFIED FIXED
Last year
Last year

People

(Reporter: roxana.leitan, Assigned: johannh)

Tracking

(Blocks 1 bug)

60 Branch
Firefox 61
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox58 wontfix, firefox59 wontfix, firefox60 verified, firefox61 verified)

Details

(Whiteboard: [storage-v2])

Attachments

(2 attachments)

Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:60.0) Gecko/20100101 Firefox/60.0
Build ID:20180301024724

[Affected versions]:
Nightly 60.0a1

[Affected platforms]:
Mac OS X 10.12, Windows 10 x64

[Preconditions]:
Have some websites in Settings list (more than 2)

[Steps to reproduce]:
1.Launch Nightly 60.0a1 with a new profile
2.Open Settings dialogue from about:preferences#privacy - Cookies and Site Data section
3.Press Shift+mouse click and select more than one website from the Settings list

[Expected result]:
The websites should be selected and highlighted

[Actual result]:
Only one website can be selected and highlighted

[Note]:
The issue is not reproducible in Exceptions dialogue list
This will be fixed by bug 1438147, but I guess it makes sense to track it individually.
Whiteboard: [storage-v2][triage]
Priority: -- → P3
Priority: P3 → P2
Summary: In Settings list can be selected only one website → Only one website in the SiteDataManager list can be selected
Priority: P2 → P3
Whiteboard: [storage-v2][triage] → [storage-v2]
Assignee: nobody → jhofmann
Status: NEW → ASSIGNED
Priority: P3 → P1
No longer depends on: 1438147
Comment hidden (mozreview-request)

Comment 4

Last year
mozreview-review
Comment on attachment 8966859 [details]
Bug 1442183 - Allow multiple selection in the site data manager list.

https://reviewboard.mozilla.org/r/235538/#review241484

Looks good to me. :)

::: browser/components/preferences/siteDataSettings.js:262
(Diff revision 1)
>      this._buildSitesList(this._sites);
>      this._list.clearSelection();
>    },
>  
>    onClickRemoveSelected() {
> -    let selected = this._list.selectedItem;
> +    for (let selected of this._list.selectedItems) {

I think we can avoid the loop here and just have this._removeSiteItems(this._list.selectedItems);
Attachment #8966859 - Flags: review?(prathikshaprasadsuman) → review+
Comment hidden (mozreview-request)
Assignee

Comment 6

Last year
mozreview-review-reply
Comment on attachment 8966859 [details]
Bug 1442183 - Allow multiple selection in the site data manager list.

https://reviewboard.mozilla.org/r/235538/#review241484

> I think we can avoid the loop here and just have this._removeSiteItems(this._list.selectedItems);

Hah, great catch, thank you!

Comment 7

Last year
Pushed by jhofmann@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/99b6961d14a2
Allow multiple selection in the site data manager list. r=prathiksha

Comment 8

Last year
bugherder
https://hg.mozilla.org/mozilla-central/rev/99b6961d14a2
Status: ASSIGNED → RESOLVED
Closed: Last year
Resolution: --- → FIXED
Target Milestone: --- → Firefox 61
Comment on attachment 8966859 [details]
Bug 1442183 - Allow multiple selection in the site data manager list.

Approval Request Comment
[Feature/Bug causing the regression]: New feature
[User impact if declined]: Slight usability issues in the site data manager as described in bug 1453589
[Is this code covered by automated tests?]: Yes
[Has the fix been verified in Nightly?]: Works in Nightly for me
[Needs manual test from QE? If yes, steps to reproduce]: Open the site data manager in about:preferences. Try to select and delete multiple sites at once (e.g. through shift+click or cmd+click).
[List of other uplifts needed for the feature/fix]: None
[Is the change risky?]: Not at all
[Why is the change risky/not risky?]: It's a very straightforward patch (a few line of code + tests) and any unlikely fallout would be contained to the front-end of that dialog.
[String changes made/needed]: None
Attachment #8966859 - Flags: approval-mozilla-beta?
Comment on attachment 8966859 [details]
Bug 1442183 - Allow multiple selection in the site data manager list.

makes sense.  approved for 60.0b13
Attachment #8966859 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Comment hidden (obsolete)
Yeah that whole test directory moved around, unfortunately. I think it's sensible to land this in beta without the test, that test is mostly meant as a regression test so that we don't lose the functionality. It should be absolutely sufficient to do a quick manual verification that multi-select works in beta.
Flags: needinfo?(jhofmann)
(I'll make a patch)
This applies cleanly on beta for me (and works). Ryan, does that work for you?

Thanks!
Flags: needinfo?(ryanvm)
Attachment #8968143 - Flags: review+
I'm not crazy about dropping the test given that we'll be living with ESR60 for the next year, but if that's the best option in your opinion, oh well I guess.
Flags: needinfo?(ryanvm)
I reproduced this issue using Fx 60.0b12, on Windows 10 x64.
I can confirm this issue is fixed, I verified using Fx 61.0a1 (build ID:20180415220108) and Fx 60.0b13, on Windows 10, mac OS 10.13 and Ubunut 14.04 LTS.
Status: RESOLVED → VERIFIED
Flags: in-testsuite+
You need to log in before you can comment on or make changes to this bug.