Closed Bug 1437880 Opened 6 years ago Closed 6 years ago

The "Remove Selected" button is not grayed out after a website is deselected

Categories

(Firefox :: Settings UI, defect, P3)

60 Branch
defect

Tracking

()

VERIFIED FIXED
Firefox 61
Iteration:
61.1 - Mar 26
Tracking Status
firefox60 --- verified
firefox61 --- verified

People

(Reporter: roxana.leitan, Assigned: accakks, Mentored)

References

(Blocks 1 open bug)

Details

(Whiteboard: [storage-v2])

Attachments

(2 files)

Attached image remove selected.png
Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:60.0) Gecko/20100101 Firefox/60.0
Build ID: 20180213100127

[Affected versions]:
Nightly 60.0a1

[Affected platforms]:
All platforms: Windows 10 x 64,Ubuntu 15.04, Mac OS X 10.12

[Preconditions]:
Have some websites in Site data Settings

[Steps to reproduce]:
1.Open Firefox
2.Go to about:preferences#privacy
3.Click Settings button from Site Data section
4.Select a website
5.Click "Site" from Settings header

[Expected result]:
The website selected in step 4 should be deselected and the "Remove Selected" button should be grayed out

[Actual result]:
The website selected in step 4 is not selected anymore but "Remove Selected" button is not grayed out
Whiteboard: [storage-v2][triage]
Priority: -- → P4
Iteration: --- → 61.1 - Mar 26
Whiteboard: [storage-v2][triage] → [storage-v2]
You can probably solve this by adding this._list.clearSelection(); to the onClickTreeCol() function here: https://searchfox.org/mozilla-central/rev/6e96a3f1e44e286ddae5fdafab737709741d237a/browser/components/preferences/siteDataSettings.js#263

Please use the steps provided in comment 0 to verify that it works as expected.
Mentor: jhofmann
Priority: P4 → P3
Assignee: nobody → aakanksha.jain8
Status: NEW → ASSIGNED
Problem is not just the button is not graying out, but when you click it, the site that was selected previously (not appearing to be selected now), get removed from the list. And this removal appears when we refresh list (example, by sorting it again).

this._list.clearSelection(); is not working here.

I'm working on this, just might take a little more time to find the solution.
It works now! :P Submitting for the review in a minute!
Attachment #8961772 - Flags: review?(prathikshaprasadsuman)
Comment on attachment 8961772 [details]
Bug 1437880 -Disable "Remove Selected"  button in the site data manager dialog if a website is deselected.

https://reviewboard.mozilla.org/r/230618/#review236414

The change looks great. Thank you!

Please upload an updated patch after amending your commit message to make it sound imperative and to include the '-'. Something like:

"Bug 1437880 - Disable Remove Selected button in the site data manager dialog if a website is deselected. r?prathiksha"

Since this is a pretty simple patch, I don't think it needs a try run. You can run the site data tests locally if you like, to check if anything is failing. Here's how you can run the tests locally:

./mach mochitest browser/components/preferences/in-content/tests/siteData/
Attachment #8961772 - Flags: review?(prathikshaprasadsuman) → review+
Let's do a try run to be sure... :)
Looking good, would you like to set the checkin-needed flag? :)
Flags: needinfo?(aakanksha.jain8)
(In reply to Johann Hofmann [:johannh] from comment #8)
> Looking good, would you like to set the checkin-needed flag? :)

Ye
Flags: needinfo?(aakanksha.jain8)
Keywords: checkin-needed
Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/e7de3b21524c
Disable "Remove Selected"  button in the site data manager dialog if a website is deselected. r=prathiksha
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/e7de3b21524c
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 61
Should we uplift this to beta?
Flags: needinfo?(jhofmann)
Comment on attachment 8961772 [details]
Bug 1437880 -Disable "Remove Selected"  button in the site data manager dialog if a website is deselected.

Yeah, why not :)

Approval Request Comment
[Feature/Bug causing the regression]: Was always broken
[User impact if declined]: Sorting site data leaves the "remove" button in a weird state
[Is this code covered by automated tests?]: Generally yes, this specific case not really
[Has the fix been verified in Nightly?]: It works in Nightly for me
[Needs manual test from QE? If yes, steps to reproduce]: see comment 0
[List of other uplifts needed for the feature/fix]: None
[Is the change risky?]: No
[Why is the change risky/not risky?]: A simple change that clears the list selection when sorting, only affects that specific dialog
[String changes made/needed]: None
Flags: needinfo?(jhofmann)
Attachment #8961772 - Flags: approval-mozilla-beta?
Comment on attachment 8961772 [details]
Bug 1437880 -Disable "Remove Selected"  button in the site data manager dialog if a website is deselected.

Trivial UI fix, approved for 60.0b11.
Attachment #8961772 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Flags: qe-verify+
Verified fixed using the latest Nightly 61.0a1 (2018-04-11) and latest Beta 60.0b11 on Windows 10 x64, Mac OS X 10.13 and Ubuntu 16.04 x64.
Status: RESOLVED → VERIFIED
Flags: qe-verify+
Flags: in-qa-testsuite+
You need to log in before you can comment on or make changes to this bug.