"Advanced" button on certificate errors doesn't work on cert errors in frames or badStsCert - @hidden vs. style.visibility = "hidden"

VERIFIED FIXED in Firefox 44

Status

()

P1
normal
VERIFIED FIXED
3 years ago
3 years ago

People

(Reporter: MattN, Assigned: bgrins)

Tracking

(Blocks: 1 bug, {regression})

unspecified
Firefox 45
regression
Points:
---
Dependency tree / graph
Bug Flags:
qe-verify +

Firefox Tracking Flags

(firefox44 verified, firefox45 verified)

Details

(Whiteboard: [fxprivacy], URL)

Attachments

(1 attachment)

I received sec_error_unknown_issuer on a site in a frame and the "Advanced" button wasn't functional.

The advanced section had @hidden=true but the button was only toggling `style.visibility` on the contents.

I suspect this is a regression from bug 1207107 as it touched related code (visibility/hidden).
status-firefox44: --- → ?
status-firefox45: --- → affected
(Assignee)

Comment 2

3 years ago
According to a comment in aboutCertError this is attempting to disallow an override: https://dxr.mozilla.org/mozilla-central/source/browser/base/content/aboutcerterror/aboutCertError.xhtml#94-96.

Instead of hiding the advancedPanel I think we should be hiding the exceptions button.  This will more closely match the behavior before the UI change.
(Assignee)

Updated

3 years ago
Summary: "Advanced" button on certificate errors doesn't work anymore @hidden vs. style.visibility = "hidden" → "Advanced" button on certificate errors doesn't work on cert errors in frames or badStsCert - @hidden vs. style.visibility = "hidden"
(Assignee)

Comment 3

3 years ago
Created attachment 8682119 [details]
MozReview Request: Bug 1220781 - Hide the exception button, not advanced section for badStsCert or cert errors in frames;r=MattN

Bug 1220781 - Hide the exception button, not advanced section for badStsCert or cert errors in frames;r=MattN
Attachment #8682119 - Flags: review?(MattN+bmo)
Comment on attachment 8682119 [details]
MozReview Request: Bug 1220781 - Hide the exception button, not advanced section for badStsCert or cert errors in frames;r=MattN

https://reviewboard.mozilla.org/r/23971/#review21399

I don't have test pages handy to test this but it seems plausible that this fixes my issue.
Attachment #8682119 - Flags: review?(MattN+bmo) → review+
(Assignee)

Updated

3 years ago
Assignee: nobody → bgrinstead
Status: NEW → ASSIGNED
(Assignee)

Comment 5

3 years ago
(In reply to Matthew N. [:MattN] from comment #4)
> Comment on attachment 8682119 [details]
> MozReview Request: Bug 1220781 - Hide the exception button, not advanced
> section for badStsCert or cert errors in frames;r=MattN
> 
> https://reviewboard.mozilla.org/r/23971/#review21399
> 
> I don't have test pages handy to test this but it seems plausible that this
> fixes my issue.

Thanks for the review.  This case is also triggered on any cert error page in a frame, so this should reproduce it also: data:text/html,<iframe src='https://expired.badssl.com'></iframe>

Try push: https://treeherder.mozilla.org/#/jobs?repo=try&revision=205a3e68000f
(Assignee)

Updated

3 years ago
status-firefox44: ? → affected
(Assignee)

Updated

3 years ago
Iteration: --- → 45.1 - Nov 16
Priority: -- → P1
Whiteboard: [fxprivacy] [triage] → [fxprivacy]

Updated

3 years ago
Flags: qe-verify?
(Assignee)

Updated

3 years ago
Flags: qe-verify? → qe-verify+

Comment 7

3 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/7ec97368b2d6
Status: ASSIGNED → RESOLVED
Last Resolved: 3 years ago
status-firefox45: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 45
QA Contact: paul.silaghi

Comment 8

3 years ago
bugherderuplift
https://hg.mozilla.org/releases/mozilla-b2g44_v2_5/rev/7ec97368b2d6
status-b2g-v2.5: --- → fixed
(Assignee)

Comment 9

3 years ago
Comment on attachment 8682119 [details]
MozReview Request: Bug 1220781 - Hide the exception button, not advanced section for badStsCert or cert errors in frames;r=MattN

Approval Request Comment
[Feature/regressing bug #]: 1207107
[User impact if declined]: User won't be able to view advanced details for non-overridable cert errors
[Describe test coverage new/current, TreeHerder]: New test case
[Risks and why]: Changing the selector of an element that's getting hidden in JS on the cert error page and that element will always exist (from the HTML), so it's not a very big risk.
[String/UUID change made/needed]:
status-b2g-v2.5: fixed → ---
Attachment #8682119 - Flags: approval-mozilla-aurora?
Verified fixed FF 45.0a1 (2015-11-04) Win 7
Status: RESOLVED → VERIFIED
status-firefox45: fixed → verified
Comment on attachment 8682119 [details]
MozReview Request: Bug 1220781 - Hide the exception button, not advanced section for badStsCert or cert errors in frames;r=MattN

The fix has been verified on Nightly, let's uplift to Aurora44.
Attachment #8682119 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+

Comment 12

3 years ago
bugherderuplift
https://hg.mozilla.org/releases/mozilla-aurora/rev/c3eed72d9e20
status-firefox44: affected → fixed
Verified fixed FF 44.0a2 (2015-11-11) OS X 10.11
status-b2g-v2.5: fixed → ---
status-firefox44: fixed → verified
You need to log in before you can comment on or make changes to this bug.