Closed Bug 1465017 Opened 2 years ago Closed 2 years ago

Safe Browsing - "See details" button not functioning

Categories

(Core :: DOM: Security, defect)

All
Unspecified
defect
Not set

Tracking

()

VERIFIED FIXED
mozilla62
Tracking Status
firefox-esr52 --- unaffected
firefox-esr60 --- unaffected
firefox60 --- unaffected
firefox61 + verified
firefox62 + verified

People

(Reporter: vlucaci, Assigned: RyanVM)

References

Details

(Keywords: regression)

Attachments

(2 files)

[Affected versions]:
- 61.0b9 (64-bit)
- 60.0
- 62.0a1 (2018-05-28) 

[Affected platforms]:

- Windows 10x64
- macOSX 10.13
- Ubuntu 16.04

[Steps to reproduce]:

1. Launch firefox

2. Open a page that is not blocked by google 
( http://itisatrap.org/firefox/its-a-trap.html )

3. Click on "See details" button.

[Expected result]:

- Clicking on the "See details" button should break down a new section containing information of the blocked page and close it if pressed again.

[Actual result]:

- The information is already displayed when reaching the block screen, rendering the "See details" button without any functionality.

[Regression range]:

- This seems to be a recent regression : https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=95f15c1c2af35c92431b86588fd9aa70553123fa&tochange=7eb059869ed692e1dbe5ade88edb0aa7eff61f1d
Christoph, can you please take a look at this (or re-assign) since Vinoth is on PTO for the next couple weeks AFAICT?
Blocks: 1452604
Flags: needinfo?(ckerschb)
Keywords: regression
(In reply to Vlad Lucaci (:vlucaci) from comment #0)
> 2. Open a page that is not blocked by google 
> ( http://itisatrap.org/firefox/its-a-trap.html )

This is also reproduceable on pages blocked by Google:

http://testsafebrowsing.appspot.com/s/phishing.html
http://testsafebrowsing.appspot.com/s/malware.html
Component: Safe Browsing → General
Product: Toolkit → Firefox
Looking at the regression range it was pretty obvious at first that the CSP applied to about:blocked causes this problem. I applied the attached patch which removes the meta CSP from about:blocked, but the problem remains. There was also no CSP message in the console that something would have been blocked which makes me think that something else causes this problem. Maybe I was doing something wrong?

Francois, using the attached patch, does the problem remain? I would be fine with temporarily removing the meta CSP from about:blocked, which is probably also the better strategy given that Bug 1452604 is already in Beta.

Vino can then take a look at the real problem once he is back.

FWIW, here are the errors in the console I see:
NS_ERROR_FAILURE: Component returned failure code: 0x80004005 (NS_ERROR_FAILURE) [nsIDocShell.tabChild] PageStyleHandler.jsm:55
[Show/hide message details.] TypeError: this.browser is null[Learn More] nsContextMenu.js:247:5
[Show/hide message details.] TypeError: gContextMenu is null[Learn More] browser.xul:1:97
NS_ERROR_FAILURE: Component returned failure code: 0x80004005 (NS_ERROR_FAILURE) [nsIDocShell.tabChild] PageStyleHandler.jsm:55
[Show/hide message details.] formatURLPref: Couldn't get pref: browser.safebrowsing.provider.test.reportPhishMistakeURL nsURLFormatter.js:123
Flags: needinfo?(ckerschb) → needinfo?(francois)
Component: General → DOM: Security
Product: Firefox → Core
(In reply to Christoph Kerschbaumer [:ckerschb, back on June 4th] from comment #3)
> Francois, using the attached patch, does the problem remain? I would be fine
> with temporarily removing the meta CSP from about:blocked, which is probably
> also the better strategy given that Bug 1452604 is already in Beta.

Yes, it's still broken after applying your patch.

I tested http://testsafebrowsing.appspot.com/s/phishing.html and the details pane shows up immediately. The "See details" button doesn't do anything.

No messages in the devtools or browser consoles.
Flags: needinfo?(francois)
Ryan, given comment 3 and comment 5, how should we move forward here? Seems like an important bug to get fixed to me!
Flags: needinfo?(ryanvm)
I was also able to bisect this to rev b5415b0216c7 (bug 1452604) with mozregression. If the smaller backout patch doesn't work, I think we should just backout the entire patch and reopen the bug.
Flags: needinfo?(ryanvm)
(In reply to Ryan VanderMeulen [:RyanVM] from comment #6)
> I was also able to bisect this to rev b5415b0216c7 (bug 1452604) with
> mozregression. If the smaller backout patch doesn't work, I think we should
> just backout the entire patch and reopen the bug.

Yes, I am fine with that approach. It seems it should be fairly straight forward to backout this changeset:
  https://phabricator.services.mozilla.com/D880

Can you do that for us Ryan?
Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/9a7562fe4671
Revert commit b5415b0216c7 (bug 1452604) for broken "See Details" button on the blocked site page.
Flags: qe-verify+
https://hg.mozilla.org/mozilla-central/rev/9a7562fe4671
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla62
Assignee: nobody → ryanvm
Hello guys,
This issue has been reproduced on the following build:
-61.0b9 (64-bit) on Windows 10x64


Confirming this issue as verified and fixed on the following platforms and builds:
- 61.0b11
- 62.0a1 (2018-06-06)

Platforms: 
- macOS 10.13
- Windows 10x64
- Ubuntu 16.04x64
Status: RESOLVED → VERIFIED
Flags: qe-verify+
You need to log in before you can comment on or make changes to this bug.