1278735 - JS doesn't work in private tabs post Bug 1269361 with NoScript installed (and "Allow [domain]" can't be used - NoScript incorrectly thinks scripts are being allowed)

Status: RESOLVED FIXED

(WebExtensions :: General, defect)

Description

[:Cykesiopka]

STR:
1. Start up any Nightly with the changes from Bug 1269361 included, using a
   fresh profile.
2. Install NoScript 2.9.0.11 or 2.9.0.12rc1 and restart.
3. Visit https://www.youtube.com/watch?v=hxUAntt1z2c in a normal tab.
   (any page which is different with JS disabled should also work, but YouTube
    pages make it very visually obvious when something is broken).
4. Perform step 3, but with a private tab.

ER:
Step 3: The video should play. Thumbnails should appear on the right.
Step 4: The video should play. Thumbnails should appear on the right.

AR:
Step 3: The video plays. Thumbnails appear on the right.
Step 4: The video doesn't play. Thumbnails don't appear on the right.

No idea of relevance, but e10s status does not make a difference.

Comment 1

[:Cykesiopka]

> 18:18.97 INFO: Last good revision: 16b4946069a9470a94f33791de7440966844747d
> 18:18.97 INFO: First bad revision: ff298d2993a32aeaca0eacac32d8e673298cf5c5
> 18:18.97 INFO: Pushlog:
> https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=16b4946069a9470a94f33791de7440966844747d&tochange=ff298d2993a32aeaca0eacac32d8e673298cf5c5
> 
> 18:19.23 INFO: Looks like the following bug has the changes which introduced the regression:
> https://bugzilla.mozilla.org/show_bug.cgi?id=1269361

As a sanity check, I also did local m-i builds:
16b4946069a9 (one revision before): Good
ff298d2993a3: Bad

Comment 2

[:Cykesiopka]

James: Does this seem like an issue with the Bug 1269361 code? If not, then maybe we can morph this bug into a Tech Evangelism bug against NoScript.

Thanks.

Comment 3

[(no longer active)]

This may be caused by NoScript relying on OriginAttributes somehow...  Anyway, I think Giorgio is a better person to investigate.

Comment 5

[Daniel Holbert [:dholbert]]

Broadening summary a bit, for searchability. (after filing bug 1279945 and not coming across this bug when doing a brief search)

Comment 6

[James Andreou]

This bug is most likely caused by interaction by NoScript and OriginAttributes. As Ehsan said, Giorgio would be the best person to investigate!

Comment 7

[Giorgio Maone [:ma1]]

NoScript does not rely on OriginAttributes AFAIK, but I'm investigating this nonetheless. 
I'll let you know as soon as I figure it out.

Comment 9

[(no longer active)]

(In reply to Giorgio Maone [:mao] from comment #7)
> NoScript does not rely on OriginAttributes AFAIK, but I'm investigating this
> nonetheless. 

Do you persist principals somehow, or use the origin suffix or such by any chance?

> I'll let you know as soon as I figure it out.

Great, thanks!

Comment 10

[Ed Morley [:emorley]]

(In reply to :Ehsan Akhgari (out sick) from comment #9)
> (In reply to Giorgio Maone [:mao] from comment #7)
> > NoScript does not rely on OriginAttributes AFAIK, but I'm investigating this
> > nonetheless. 
> 
> Do you persist principals somehow, or use the origin suffix or such by any
> chance?

Comment 11

[David Bolter [:davidb] (NeedInfo me for attention)]

I'm looking at regressions for FF 49. What is the impact of this bug? Do we need to fix this before release (or consider any backouts)?

Comment 12

[Giorgio Maone [:ma1]]

I've found the culprit, being an usage of principal.origin instead of principal.originNoSuffix while matching JavaScript permissions, which are mapped to "legacy" origin strings.
Fixing it in next release, hopefully this very week-end.

Comment 13

[Daniel Holbert [:dholbert]]

That's great news -- thanks Giorgio!

I'll reply to David's questions:
(In reply to David Bolter [:davidb] from comment #11)
> What is the impact of this bug?

If you have the NoScript add-on installed, Private Browsing becomes unusable, for any sites that require some JS. (All website-provided JS gets blocked, with no way to unblock.  Moreover, NoScript's UI confusingly says all scripts are allowed.)

> Do we need to fix this before release

If at all possible, yes.  (Though it's NoScript add-on code that needs to be updated to fix this, to accommodate the expanded OriginAttributes that we added in bug 1269361. Happily, it sounds like a fix is close!)

> (or consider any backouts)?

If we were to backout to fix this, we'd be backing out bug 1269361. That was a decently large patch, and it might not be easy/safe to back it out at this point.

Comment 14

[Giorgio Maone [:ma1]]

Fixed in NoScript 2.9.0.12rc2, thank you.
https://noscript.net/getit#devel

Comment 15

[:Cykesiopka]

Thanks!

Fixed for me using NoScript 2.9.0.12rc2 and Nightly 50 and Aurora 49 => FIXED, but feel free to reopen if needed.

Comment 16

[Paul [pwd]]

Verifying.

Comment 17

[David Bolter [:davidb] (NeedInfo me for attention)]

Thanks everyone!