1488361 - Clear Data / Manage Data (cookies and site data) buttons are deactivated on first run

Status: VERIFIED FIXED

(Firefox :: Settings UI, defect, P1)

Description

[Tom S]

Steps to reproduce:
1. Compile mozilla-central
2. run ./mach run
3. Go to about:preferences#privacy
4. Observe that "Clear Data" and "Manage Data" are gray and can't be clicked
5. Reload the page
6. Buttons are clickable

Comment 1

[Johann Hofmann [:johannh]]

Considering that we landed a ton of patches in preferences yesterday, I assume this was broken by the third party cookies work...

I'll try to find the regression...

Comment 2

[Johann Hofmann [:johannh]]

[Tracking Requested - why for this release]:
Users can not clear site data in about:preferences when browser.contentblocking.ui is true

Ehsan, this seems to have been caused by bug 1488310, probably some race/timing issue with moving the site data section. We should look into it.

I can confirm that nothing is broken when browser.contentblocking.ui is set to false, so beta users are not going to immediately be impacted by this, but we should still fix this in a timely manner.

Comment 3

[Johann Hofmann [:johannh]]

We should also figure out why tests are not catching this...

Comment 4

[(no longer active)]

I can't reproduce this.  Better steps to reproduce are appreciated.

Based on the code, the *only* possible case that might be causing this (as in, the only code that disables both of these buttons) is something being clearing the site data in the background! <https://searchfox.org/mozilla-central/rev/c3fef66a5b211ea8038c1c132706d02db408093a/browser/components/preferences/in-content/privacy.js#1944>  That seems completely unrelated to all of the recent changes that have landed in the past day.

Also, I don't understand the connection to bug 1488310 (that was duped against bug 1488436 right now), was that just a guess?  May I ask what based your suspicion?

Comment 5

[Takanori MATSUURA]

In my case, "Download Links" WebExtension make this issue.
https://addons.mozilla.org/ja/firefox/addon/download-links/

When the WebExtension is disabled, Clear Data / Manage Data buttons are activated.

Comment 6

[Takanori MATSUURA]

(In reply to Takanori MATSUURA from comment #5)

Sorry. Comment #5 is incorrect.
Enable/Disable WebExtensions are not related this issue.

* Type "about:preferences#privacy" directly into the new tab, Clear Data / Manage Data buttons are active.
* Visit "about:preferences" and select "Privacy & Security", Clear Data / Manage Data buttons are inactive.

Comment 7

[Peter Magyari (Desktop QA)]

Hello,

Here is the Regression Range I found for this issue:

INFO: Last good revision: 75411571f3a4aeabbc1697b5dd7f4f827af3d29d
INFO: First bad revision: 8351581ba5a44684bdfbbcf731756cd5f785c46c

INFO: Pushlog: https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=75411571f3a4aeabbc1697b5dd7f4f827af3d29d&tochange=8351581ba5a44684bdfbbcf731756cd5f785c46c

Comment 8

[Alice0775 White]

(In reply to :Ehsan Akhgari from comment #4)
> I can't reproduce this.  Better steps to reproduce are appreciated.
> 


Steps to reproduce:
1. Launch Nightly
2. Open about:preferences#privacy
  --- Bug appears
3. Reload (f5)
  --- Bug Disappears

4. Re-launch Nightly
5. Open about:preferences#privacy
  --- Bug appears again

Comment 9

[(no longer active)]

I have tried everything I can possibly think of, and I still cannot reproduce this bug.

So I resorted to reading the code a bit more, and I have a theory as to what may be going on here.  First note how this code starts to update the sites immediately after setting up the observer notifications: <https://searchfox.org/mozilla-central/rev/c3fef66a5b211ea8038c1c132706d02db408093a/browser/components/preferences/in-content/privacy.js#410> and also note that SiteDataManager.updateSites() immediately dispatches the "sitedatamanager:updating-sites" notification <https://searchfox.org/mozilla-central/rev/5a18fb5aeeec99f1ca1c36a697082c221189a3b9/browser/modules/SiteDataManager.jsm#48>.  So the two buttons are disabled immediately after initialization.

Now, it is possible that the sitedatamanager:sites-updated notification is arriving before the XBL binding has been reattached to the elements, which would cause this code <https://searchfox.org/mozilla-central/source/browser/components/preferences/in-content/privacy.js#1949> to just set an expando "disabled" property on the node that has no effect whatsoever, in a very similar vain to what was happening in bug 1488013.

In order to avoid this whole mess of a race condition, we can simply start the initialization of these buttons after we've moved the UI elements, I think.

Comment 10

[(no longer active)]

Attachment: Bug 1488361 - Only initiate the updating of site data control buttons after we have moved the buttons in the UI; r=johannh

Comment 11

[Alice0775 White]

FYI,
With opt build (Windows 2012 x64 opt), I can reproduce the problem with 100% probability.

On the other hand, 
With debug build (Windows 2012 x64 debug), I can reproduce this problem with approx 80% probability. And sometimes I cannot reproduce.

Comment 12

[Johann Hofmann [:johannh]]

Comment on attachment 9006630 [details]
Bug 1488361 - Only initiate the updating of site data control buttons after we have moved the buttons in the UI; r=johannh

Johann Hofmann [:johannh] has approved the revision.

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/a0e69b970c12
Only initiate the updating of site data control buttons after we have moved the buttons in the UI; r=johannh

Comment 14

[(no longer active)]

(In reply to Alice0775 White from comment #11)
> FYI,
> With opt build (Windows 2012 x64 opt), I can reproduce the problem with 100%
> probability.
> 
> On the other hand, 
> With debug build (Windows 2012 x64 debug), I can reproduce this problem with
> approx 80% probability. And sometimes I cannot reproduce.

Thanks a lot, Alice.  Johann could reproduce the problem and he verified that this patch really fixes the issue.  :-)

Comment 15

[Tiberius Oros[:tiberius_oros]]

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

Comment 16

[(no longer active)]

Comment on attachment 9006630 [details]
Bug 1488361 - Only initiate the updating of site data control buttons after we have moved the buttons in the UI; r=johannh

Approval Request Comment
[Feature/Bug causing the regression]: Bug 1488013
[User impact if declined]: See comment 0
[Is this code covered by automated tests?]: No.
[Has the fix been verified in Nightly?]: It has been verified locally.
[Needs manual test from QE? If yes, steps to reproduce]: Yes, comment 0.
[List of other uplifts needed for the feature/fix]: None.
[Is the change risky?]: Not too risky.
[Why is the change risky/not risky?]: The code change is rather small and localized, and is disabled by default.
[String changes made/needed]: None.

Comment 17

[Pascal Chevrel:pascalc]

Comment on attachment 9006630 [details]
Bug 1488361 - Only initiate the updating of site data control buttons after we have moved the buttons in the UI; r=johannh

Uplift approved for 63 beta 5

Comment 18

[Eliza Balazs [:ebalazs_]]

https://hg.mozilla.org/releases/mozilla-beta/rev/a14dab7cea37

Comment 19

[Adrian Florinescu [:aflorinescu]]

I've also encountered reproducibility issues on Nightly 64 affected build(09/04/2018), but managed to reproduce it. (the trick is new profile)

Pending the beta uplifted, verified as fixed on Nightly 64 09/07/2018 Windows 10, Ubuntu 16.04 and Osx 10.13.

Comment 20

[Adrian Florinescu [:aflorinescu]]

Verified as fixed Windows 10, Ubuntu 16.04 and Osx 10.13:
63.0b6 	20180913141435