Closed Bug 1220781 Opened 4 years ago Closed 4 years ago

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

Categories

(Firefox :: General, defect, P1)

defect

Tracking

()

VERIFIED FIXED
Firefox 45
Iteration:
45.1 - Nov 16
Tracking Status
firefox44 --- verified
firefox45 --- verified

People

(Reporter: MattN, Assigned: bgrins)

References

(Blocks 1 open bug, )

Details

(Keywords: regression, Whiteboard: [fxprivacy])

Attachments

(1 file)

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).
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.
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"
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: nobody → bgrinstead
Status: NEW → ASSIGNED
(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
Iteration: --- → 45.1 - Nov 16
Priority: -- → P1
Whiteboard: [fxprivacy] [triage] → [fxprivacy]
Flags: qe-verify?
Flags: qe-verify? → qe-verify+
https://hg.mozilla.org/mozilla-central/rev/7ec97368b2d6
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 45
QA Contact: paul.silaghi
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]:
Attachment #8682119 - Flags: approval-mozilla-aurora?
Verified fixed FF 45.0a1 (2015-11-04) Win 7
Status: RESOLVED → 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+
Verified fixed FF 44.0a2 (2015-11-11) OS X 10.11
You need to log in before you can comment on or make changes to this bug.