1514070 - Cookies subpanel continues to show that a cookie is "Blocked" after it has received an exception

Status: VERIFIED FIXED

(Firefox :: Protections UI, defect, P1)

Description

[Steven Englehardt [:englehardt]]

Attachment: Screenshot 2018-12-13 15:04:56.png

STR:

1. On a fresh profile open https://senglehardt.com/test/identity_providers/google_custom.html.
2. Click "Google", which should trigger a popup from accounts.google.com.
3. Interact with the pop-up window (enter some text, click around)
4. Open the cookies subpanel.

Expected result:
accounts.google.com row says "Allowed" and is not grayed out.

Actual result:
accounts.google.com row says "Blocked", is grayed out, but contains an X icon that indicates the origin (correctly) has an exception.

When I refresh the page, I see the expected result.

I've observed this both on Nightly (66.0a1 - 20181213094243) and Beta (65.0b4 - 20181211223337).

Comment 1

[(no longer active)]

If you load https://senglehardt.com/test/identity_providers/google_custom.html in a new tab right after, you'll see the correct label (Allowed.)

The content blocking logs in the first and second tabs, respectively:

await gBrowser.selectedBrowser.getContentBlockingLog()
"{
 \"https://senglehardt.com\": [[32768, true, 1], [536870912, true, 1]],
 \"https://apis.google.com\": [[8192, true, 1], [32768, true, 1], [536870912, true, 1]],
 \"https://ssl.gstatic.com\": [[8192, true, 1]],
 \"https://accounts.google.com\": [[8192, true, 1], [32768, true, 1], [536870912, true, 175], [536870912, false, 1]]
}
"

await gBrowser.selectedBrowser.getContentBlockingLog()
"{
 \"https://senglehardt.com\": [[32768, true, 1]],
 \"https://apis.google.com\": [[8192, true, 1]],
 \"https://ssl.gstatic.com\": [[8192, true, 1]],
 \"https://accounts.google.com\": [[8192, true, 1], [32768, true, 1]]
}
"

Comment 2

[(no longer active)]

Attachment: Bug 1514070 - Ensure that the cookies subpanel will correctly show granted permissions to trackers as soon as they're granted

Comment 3

[Tanvi Vyas[:tanvi]]

Does this need uplift?

Comment 4

[(no longer active)]

(In reply to Tanvi Vyas[:tanvi] from comment #3)
> Does this need uplift?

Once it gets reviewed and landed, yes.

Comment 5

[(no longer active)]

Comment on attachment 9031468 [details]
Bug 1514070 - Ensure that the cookies subpanel will correctly show granted permissions to trackers as soon as they're granted

[Beta/Release Uplift Approval Request]

Feature/Bug causing the regression: Bug 1501992

User impact if declined: See comment 0

Is this code covered by automated tests?: Yes

Has the fix been verified in Nightly?: No

Needs manual test from QE?: No

If yes, steps to reproduce: 

List of other uplifts needed: None

Risk to taking this patch: Low

Why is the change risky/not risky? (and alternatives if risky): Quite low risk, very simple code change

String changes made/needed: None

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/7bebed7c135a
Ensure that the cookies subpanel will correctly show granted permissions to trackers as soon as they're granted r=ewright

Comment 7

[Bogdan Tara[:bogdan_tara | bogdant]]

https://hg.mozilla.org/mozilla-central/rev/7bebed7c135a

Comment 8

[Vlad Lucaci, Thunderbird QA (:vlucaci)]

Hello all,

This issue is verified fixed on the following build 66.0a1 (2018-12-19)(20181219045530) on Windows 10x64, macOS10.13 and Ubuntu 16.04x64.

Comment 9

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9031468 [details]
Bug 1514070 - Ensure that the cookies subpanel will correctly show granted permissions to trackers as soon as they're granted

[Triage Comment]
Needed for the Control Center UI changes shipping in 65. Approved for 65.0b6.

Comment 10

[Ryan VanderMeulen [:RyanVM]]

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

Comment 11

[Vlad Lucaci, Thunderbird QA (:vlucaci)]

Hello all, 
I have tried to verify this issue again, but it does not seem to be fixed in Beta 65.0b6 nor 65.0b7(taskcluster). However it is present on the latest 66.0a1 Nightly build.

Anyone know anything about this?

Comment 12

[(no longer active)]

This is certainly fixed.  Did you check the value of the network.cookie.cookieBehavior pref when you were testing?  Perhaps you were confused since bug 1514853 mistakenly reverted that pref to 0 on Nightly and Beta on Friday.

Comment 13

[Vlad Lucaci, Thunderbird QA (:vlucaci)]

Hello Ehsan,

I tried again while checking the network.cookie.cookieBehavior pref in about:config. 

When the issue is reproduced as per the Description, the pref default value is set to 4.

I tried to reproduce this issue again and set the value to 0. When I do this, then I get the desired Expected result:

"accounts.google.com row says "Allowed" and is not grayed out."

Comment 14

[(no longer active)]

That happens because you were testing the cookieBehavior=0 case in the same profile as the cookieBehavior=4 case, so the Allowed label comes from the information that the previous test leaves in the profile, that is why the expected results match.  (That information persists for 30 days in your profile.)

If you repeat the test with a new profile with cookieBehavior=0, you will see that the Cookies subpanel wouldn't show an Allowed label next to accounts.google.com, which is the expected behaviour with cookieBehavior=0.

(Note that doing that test isn't really necessary, just noting the above to answer your question.)

Comment 15

[Vlad Lucaci, Thunderbird QA (:vlucaci)]

Hello,

Indeed, when I start the browser and go to about:config and change the value of cookieBehavior to 0, I get the desired behavior. 

But the problem here is that 0 is not the default value upon first launch on fresh profile. The default value is 4, and this is why the issue still reproduces as per in Description.

Comment 16

[(no longer active)]

(In reply to Vlad Lucaci (:vlucaci) from comment #15)
> Hello,
> 
> Indeed, when I start the browser and go to about:config and change the value
> of cookieBehavior to 0, I get the desired behavior. 
> 
> But the problem here is that 0 is not the default value upon first launch on
> fresh profile.

0 is the default on beta now, see bug 1514853 (Dev Edition, Beta and Release).

> The default value is 4, and this is why the issue still
> reproduces as per in Description.

Again, as I mentioned in comment 14, what appears in this row has *nothing to do* with the value of the cookieBehavior pref at all.  It has to do with what has happened in the profile before.  There are two cases:

A) The user has never done anything that has caused accounts.google.com to receive an "Allowed" permission on the test website (https://senglehardt.com) here.  In this case, there will be no Allowed label next to the row corresponding to accounts.google.com.

B) The user has previously done something(*) that has caused accounts.google.com to receive an "Allowed" permission on the test website (https://senglehardt.com) here.  In this case, there will be an Allowed label next to the row corresponding to accounts.google.com.

As you can see, the value of the cookieBehavior pref is never consulted here at all.

The bug (which has been fixed now) was that right after the "something" above, we would mistakenly show the label as "Blocked" instead of "Allowed" in the UI until you refreshed the page and it was merely a UI issue (in other words, we were showing the wrong string in the UI.)


The expected result in comment 0 has been written with the expectation that you follow the exact STR in that comment on Nightly.  Nobody has claimed anywhere that if you do other things (such as flipping the cookieBehavior pref) you will get exactly the same result.  :-)  At any rate, Firefox is just telling the user what's happening here when the cookieBehavior pref is 0 in the scenario that you are testing, which is, that accounts.google.com has an Allowed permission on this website, and the user can clear if they choose to do so, and that cookie access has been permitted on the website accordingly.  Everything is acting as intended here.  Hope this helps clarify things...


(*) The "something" here is interacting with the accounts.google.com popup window that opens in the STR in comment 0, for example clicking somewhere on it or typing something in the text box.

Comment 17

[Vlad Lucaci, Thunderbird QA (:vlucaci)]

>0 is the default on beta now, see bug 1514853 (Dev Edition, Beta and Release).

When you first launch the latest Beta , the default value is actually 4 , not 0.

>The "something" here is interacting with the accounts.google.com popup window that opens in the STR in comment 0, for example clicking somewhere on it or typing something in the text box.

I have attached a screen cast of the issue. As you can see, I am interacting with the accounts.google.com popup window that opens, and I still have it displayed with the wrong string.

When trying in the latest Nightly, the issue is indeed no longer present.

https://drive.google.com/file/d/1gTBWl2wRITtPAmpfl7Kq_IlotDOrsbyS/view?usp=sharing

Comment 18

[(no longer active)]

(In reply to Vlad Lucaci (:vlucaci) from comment #17)
> >0 is the default on beta now, see bug 1514853 (Dev Edition, Beta and Release).
> 
> When you first launch the latest Beta , the default value is actually 4 ,
> not 0.

My apologies, I thought the current beta was beta 8 but after I checked now I realized we are still at beta 7.  Beta 7 is an "early beta", which means the EARLY_BETA_OR_EARLIER macro is still defined for beta7 builds, therefore the value 4 is being used: <https://searchfox.org/mozilla-central/source/browser/app/profile/firefox.js#1521>.  Beta7 is considered the last "early beta" (but let's not be pedantic about it, and after that the beta builds are considered late beta and the EARLY_BETA_OR_EARLIER macro will not be defined, therefore this line will be #ifdef'ed out of firefox.js, so the value 0 will be used after that point, all the way through to the release builds.  We have been flipping this pref back to value 0 on the beta channel using this same mechanism since the 63 cycle, and this mechanism has been working well for years.  There is no need to doubt that it is going to happen here too.

> >The "something" here is interacting with the accounts.google.com popup window that opens in the STR in comment 0, for example clicking somewhere on it or typing something in the text box.
> 
> I have attached a screen cast of the issue. As you can see, I am interacting
> with the accounts.google.com popup window that opens, and I still have it
> displayed with the wrong string.
> 
> When trying in the latest Nightly, the issue is indeed no longer present.
> 
> https://drive.google.com/file/d/1gTBWl2wRITtPAmpfl7Kq_IlotDOrsbyS/
> view?usp=sharing

Sorry, I don't know what version you are testing here, but this screencast has been taken from a build before this test: <https://my.mixtape.moe/dqydog.webm>.  This bug has absolutely been fixed on both Nightly and Beta.  This video evidence is taken from the beta build that I just downloaded from https://www.mozilla.org/en-CA/firefox/channel/desktop/ in a new profile, first showing the about:support page so that you can verify the build ID to ensure nothing funny is going on.

Comment 19

[Vlad Lucaci, Thunderbird QA (:vlucaci)]

Thank you for the clarifications Ehsan.