1201365 - Remove offline data in the advanced preferences is broken

Status: VERIFIED FIXED

(Firefox :: Settings UI, defect)

Comment 1

[Marco Castelluccio [:marco]]

Attachment: fix_clear_offline_data

Comment 2

[Jared Wein [:jaws] (please needinfo? me)]

Comment on attachment 8656357 [details] [diff] [review]
fix_clear_offline_data

Review of attachment 8656357 [details] [diff] [review]:
-----------------------------------------------------------------

browser/components/preferences/advanced.js was just removed today by bug 1140495. Only the in-content change is necessary.

Comment 3

[Marco Castelluccio [:marco]]

Attachment: fix_clear_offline_data

Carrying r+.

There isn't any test covering this, so I haven't sent the patch to try.

Comment 4

[Pulsebot]

https://hg.mozilla.org/integration/fx-team/rev/d3f284a63392

Comment 5

[Jared Wein [:jaws] (please needinfo? me)]

(In reply to Marco Castelluccio [:marco] from comment #3)
> Created attachment 8656476 [details] [diff] [review]
> fix_clear_offline_data
> 
> Carrying r+.
> 
> There isn't any test covering this, so I haven't sent the patch to try.

Can you please file a new bug to add tests for this? Would be great if you attached a patch there too ;)

Comment 6

[Marco Castelluccio [:marco]]

> Can you please file a new bug to add tests for this? Would be great if you attached a patch there too ;)

Filed bug 1203354. Unfortunately I don't have time to do it at the moment, but maybe will take a stab at it in the future.

Comment 7

[Marco Castelluccio [:marco]]

Should we uplift the fix to Aurora?

Comment 8

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

[Tracking Requested - why for this release]: Regression, users can't remove site from Options > Advanced > Network > Offline data.

Comment 9

[:Gijs (he/him)]

(In reply to Marco Castelluccio [:marco] from comment #7)
> Should we uplift the fix to Aurora?

Please request approval for beta (42 has merged up), yes. I don't fully understand the additional , true) argument for the clearing call - is that going to work on 42, too?

Comment 10

[Marco Castelluccio [:marco]]

(In reply to :Gijs Kruitbosch from comment #9)
> (In reply to Marco Castelluccio [:marco] from comment #7)
> > Should we uplift the fix to Aurora?
> 
> Please request approval for beta (42 has merged up), yes. I don't fully
> understand the additional , true) argument for the clearing call - is that
> going to work on 42, too?

It should work, as the third argument (exactHost) is present in http://mxr.mozilla.org/mozilla-beta/source/netwerk/base/nsIPermissionManager.idl as well.

I don't know if my patch applies cleanly to beta, I'll try tomorrow and publish a new patch in case it doesn't apply.

Comment 11

[Marco Castelluccio [:marco]]

Comment on attachment 8656476 [details] [diff] [review]
fix_clear_offline_data

Approval Request Comment
[Feature/regressing bug #]: I'm not sure what regressed this.
[User impact if declined]: Users won't be able to clear offline data for websites from the "Advanced" preferences
[Describe test coverage new/current, TreeHerder]: No test coverage.
[Risks and why]: The patch is very simple, it simply adds a missing import and a missing parameter in a function.
[String/UUID change made/needed]: None.

Comment 12

[Sylvestre Ledru [:Sylvestre]]

Regression, tracking.

Comment 13

[Carsten Book [:Tomcat]]

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

Comment 14

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/releases/mozilla-beta/rev/88cb07c61bab

Comment 15

[Vasilica Tamas, QA [:vtamas]]

I tried on several builds to reproduce this issue but without any luck. However, I've checked on Firefox 44.0a1 (2015-09-30), Firefox 43.0a2 (2015-09-30) and Firefox 42 Beta 2 on Windows 7 64-bit and the “Clear Now” button from about:preferences#advanced works as expected.

Marco, since you've reproduced this bug, could you please confirm that it's fixed now?

Comment 16

[Marco Castelluccio [:marco]]

I wrote the patch, so I tested it while working on it :D

Comment 17

[Vasilica Tamas, QA [:vtamas]]

Thanks Marco! Based on Comment 15 and Comment 16 I am marking this bug as Verified.