Closed Bug 1567561 Opened 5 years ago Closed 5 years ago

Render certificate info in about:certificate UI

Categories

(Firefox :: Security, enhancement, P1)

enhancement

Tracking

()

RESOLVED FIXED
Firefox 70
Tracking Status
firefox70 --- fixed

People

(Reporter: carolina.jimenez.g, Assigned: carolina.jimenez.g, Mentored)

References

(Blocks 1 open bug)

Details

Attachments

(1 file)

No description provided.
Status: NEW → ASSIGNED
Priority: -- → P1
Summary: Send certificate info to UI → Render certificate info in about:certerror UI
Summary: Render certificate info in about:certerror UI → Render certificate info in about:certificate UI
Attachment #9079422 - Attachment description: Bug 1567561 - Sends certificate info to UI → Bug 1567561 - Render real certificate information in about:certificate
Mentor: jhofmann
Keywords: checkin-needed
Attachment #9079422 - Attachment description: Bug 1567561 - Render real certificate information in about:certificate → Bug 1567561 - Render real certificate information in about:certificate.r=johannh

Pushed by csabou@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/9bb8277bcb2f
Render real certificate information in about:certificate.r=johannh

Keywords: checkin-needed

Push with failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&group_state=expanded&resultStatus=success%2Cpending%2Crunning%2Ctestfailed%2Cbusted%2Cexception&searchStr=browser%2Cchrome&fromchange=9bb8277bcb2f6432437fe06c856585a4c53d76f2&tochange=1800c95a395b06d3f188a25e64e2dfbafd8e1e00&selectedJob=258876327

Failure log: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=258876593&repo=autoland&lineNumber=34622

Backout link: https://hg.mozilla.org/integration/autoland/rev/7cf2e87625ff4d2ff8eba69e0dae71f273455ee0

[task 2019-07-29T21:52:25.653Z] 21:52:25 INFO - TEST-PASS | toolkit/components/certviewer/tests/browser/browser_renderCertToUI.js | data-l10n-id must contain the original label - true == true -
[task 2019-07-29T21:52:25.654Z] 21:52:25 INFO - Buffered messages finished
[task 2019-07-29T21:52:25.656Z] 21:52:25 INFO - TEST-UNEXPECTED-FAIL | toolkit/components/certviewer/tests/browser/browser_renderCertToUI.js | Info must be equal - "5/8/2018, 12:00:00 AM (Coordinated Universal Time)" == "5/7/2018, 9:00:00 PM (Brasilia Standard Time)" -
[task 2019-07-29T21:52:25.656Z] 21:52:25 INFO - Stack trace:
[task 2019-07-29T21:52:25.656Z] 21:52:25 INFO - resource://testing-common/content-task.js line 62 > eval:null:81
[task 2019-07-29T21:52:25.657Z] 21:52:25 INFO - TEST-PASS | toolkit/components/certviewer/tests/browser/browser_renderCertToUI.js | data-l10n-id must contain the original label - true == true -
[task 2019-07-29T21:52:25.659Z] 21:52:25 INFO - Not taking screenshot here: see the one that was previously logged
[task 2019-07-29T21:52:25.669Z] 21:52:25 INFO - TEST-UNEXPECTED-FAIL | toolkit/components/certviewer/tests/browser/browser_renderCertToUI.js | Info must be equal - "6/3/2020, 12:00:00 PM (Coordinated Universal Time)" == "6/3/2020, 9:00:00 AM (Brasilia Standard Time)" -
[task 2019-07-29T21:52:25.669Z] 21:52:25 INFO - Stack trace:
[task 2019-07-29T21:52:25.669Z] 21:52:25 INFO - resource://testing-common/content-task.js line 62 > eval:null:81
[task 2019-07-29T21:52:25.669Z] 21:52:25 INFO - TEST-PASS | toolkit/components/certviewer/tests/browser/browser_renderCertToUI.js | The element exists in adjustedCerts - {"sectionTitle":"Subject Alt Names","sectionItems":[{"label":"DNS Name","info":"github.com"},{"label":"DNS Name","info":"www.github.com"}],"Critical":false} == true -
[task 2019-07-29T21:52:25.669Z] 21:52:25 INFO - TEST-PASS | toolkit/components/certviewer/tests/browser/browser_renderCertToUI.js | sectionItems must be the same length - 2 == 2 -

Flags: needinfo?(carolina.jimenez.g)

uhmm that's strange, when I run toolkit/components/certviewer/tests/browser/browser_renderCertToUI.js local it passes all the tests. Thank you, I'll investigate it

Flags: needinfo?(carolina.jimenez.g)

The time/date seems to be formatted in a different locale/timezone: "6/3/2020, 12:00:00 PM (Coordinated Universal Time)" == "6/3/2020, 9:00:00 AM (Brasilia Standard Time)". UTC vs Brazil

(In reply to Tom Schuster [:evilpie] from comment #5)

The time/date seems to be formatted in a different locale/timezone: "6/3/2020, 12:00:00 PM (Coordinated Universal Time)" == "6/3/2020, 9:00:00 AM (Brasilia Standard Time)". UTC vs Brazil

Thank you for the clarification Tom, I'm working on a solution now

Depends on: 1569737
Blocks: 1568261
Keywords: checkin-needed

Pushed by cbrindusan@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/333922d27937
Render real certificate information in about:certificate.r=johannh

Keywords: checkin-needed
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 70

Backed out changeset 333922d27937 (bug 1567561) for browser-chrome failures at browser/base/content/test/performance/browser_startup_syncIPC.js

Backout: https://hg.mozilla.org/mozilla-central/rev/566622d8bd7b99311a32f39f705dc178121d2154

Failure push: https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=333922d279371588f26fff6469c2cf38c26b3b88

Failure logs:
https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=259798729&repo=autoland&lineNumber=1330
https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=259797350&repo=autoland&lineNumber=1319

[task 2019-08-03T20:12:34.517Z] 20:12:34 INFO - TEST-PASS | browser/base/content/test/performance/browser_startup_syncIPC.js | sync IPC PWebRenderBridge::Msg_GetSnapshot allowed 1 more times before handling user events -
[task 2019-08-03T20:12:34.518Z] 20:12:34 INFO - Buffered messages finished
[task 2019-08-03T20:12:34.518Z] 20:12:34 INFO - TEST-UNEXPECTED-FAIL | browser/base/content/test/performance/browser_startup_syncIPC.js | unused whitelist entry before handling user events: PWebRenderBridge::Msg_GetSnapshot -
[task 2019-08-03T20:12:34.518Z] 20:12:34 INFO - Stack trace:
[task 2019-08-03T20:12:34.518Z] 20:12:34 INFO - chrome://mochikit/content/browser-test.js:test_ok:1576
[task 2019-08-03T20:12:34.518Z] 20:12:34 INFO - chrome://mochitests/content/browser/browser/base/content/test/performance/browser_startup_syncIPC.js:null:322
[task 2019-08-03T20:12:34.518Z] 20:12:34 INFO - chrome://mochikit/content/browser-test.js:Tester_execTest/<:1346
[task 2019-08-03T20:12:34.518Z] 20:12:34 INFO - chrome://mochikit/content/browser-test.js:Tester_execTest:1381
[task 2019-08-03T20:12:34.519Z] 20:12:34 INFO - chrome://mochikit/content/browser-test.js:nextTest/<:1209
[task 2019-08-03T20:12:34.519Z] 20:12:34 INFO - chrome://mochikit/content/tests/SimpleTest/SimpleTest.js:SimpleTest.waitForFocus/waitForFocusInner/focusedOrLoaded/<:803
[task 2019-08-03T20:12:34.519Z] 20:12:34 INFO - TEST-PASS | browser/base/content/test/performance/browser_startup_syncIPC.js | sync IPC PAPZInputBridge::Msg_ProcessUnhandledEvent happened as many times as expected before handling user events -
[task 2019-08-03T20:12:34.519Z] 20:12:34 INFO - TEST-PASS | browser/base/content/test/performance/browser_startup_syncIPC.js | sync IPC PAPZInputBridge::Msg_ReceiveMouseInputEvent happened as many times as expected before handling user events -
[task 2019-08-03T20:12:34.519Z] 20:12:34 INFO - TEST-PASS | browser/base/content/test/performance/browser_startup_syncIPC.js | sync IPC PLayerTransaction::Msg_GetTextureFactoryIdentifier happened as many times as expected before handling user events -
[task 2019-08-03T20:12:34.519Z] 20:12:34 INFO - whitelisted sync IPC before becoming idle:
[task 2019-08-03T20:12:34.520Z] 20:12:34 INFO - PCompositorBridge::Msg_NotifyChildCreated - at most 1 times
[task 2019-08-03T20:12:34.520Z] 20:12:34 INFO - PAPZInputBridge::Msg_ProcessUnhandledEvent - at most 1 times
[task 2019-08-03T20:12:34.520Z] 20:12:34 INFO - PAPZInputBridge::Msg_ReceiveMouseInputEvent - at most 1 times
[task 2019-08-03T20:12:34.520Z] 20:12:34 INFO - PLayerTransaction::Msg_GetTextureFactoryIdentifier - at most 1 times
[task 2019-08-03T20:12:34.520Z] 20:12:34 INFO - PCompositorBridge::Msg_Initialize - at most 1 times
[task 2019-08-03T20:12:34.520Z] 20:12:34 INFO - PCompositorBridge::Msg_MapAndNotifyChildCreated - at most 1 times
[task 2019-08-03T20:12:34.521Z] 20:12:34 INFO - Not taking screenshot here: see the one that was previously logged

Status: RESOLVED → REOPENED
Flags: needinfo?(carolina.jimenez.g)
Resolution: FIXED → ---
Target Milestone: Firefox 70 → ---

Needinfoing myself to needinfo :florian after his bugzilla account got unlocked.

Flags: needinfo?(aryx.bugmail)

I'm sorry but what the hell?

  • Why did this test not fail on autoland? I've had this happen to me before and it's always a configuration failure. By policy can we please NOT back out patches on central that did not fail on autoland and file a follow-up bug instead? You're draining the developer's time and mental energy when it's not actually their fault or even their responsibility to clean up.
  • There is literally nothing in this patch that runs at startup. The only way this patch could have caused the test to fail is by shifting test order. In which case, again, backing out of central is really bad form.
  • Backing this out of central without a least a little bit of context in form of a comment (or trying to reach out to us), and, to top it off, the announcement to eventually needinfo florian, who has no visible connection to this bug at all and is not responsible for resolving it. If this test failure is florian's fault, please disable the test, file a new bug and assign it to him.

I'm not sure what to say. I really hope you just backed out the wrong patch accidentally. Otherwise this behavior is incredibly frustrating and disruptive, even damaging, to our work.

Flags: needinfo?(ccoroiu)

(In reply to Johann Hofmann [:johannh] from comment #11)

I'm sorry but what the hell?

You're draining the developer's time and mental energy when it's not actually their fault or even their responsibility to clean up.
In which case, again, backing out of central is really bad form.
Otherwise this behavior is incredibly frustrating and disruptive, even damaging, to our work.

"No abusing people. Constant and intense critique is one of the reasons we build great products. It's harder to fall into group-think if there is always a healthy amount of dissent. We want to encourage vibrant debate inside of the Mozilla community, we want you to disagree with us, and we want you to effectively argue your case. However, we require that in the process, you criticize things, not people. Examples of things include: interfaces, algorithms, and schedules. Examples of people include: developers, designers, and users. Attacking or encouraging attacks on a person may result in you being banned from Bugzilla. " -- https://bugzilla.mozilla.org/page.cgi?id=etiquette.html

I apologize if the harsh language was perceived as going against any particular person, that was not my intent. I know that you're acting based on a process and I find the process to be quite lacking in this case, which is what my comment aimed at.

I would still like to have a serious discussion on whether patches can be backed out of central based on failing tier-1 tests that should have run on autoland. It is, again criticizing the process, highly disruptive to a developer's work to have their patches backed out of central. In my opinion this practice should be left for serious Nightly regressions or at the developer's own request.

The test did almost permafail on autoland but it has a matching suggestion (bug 1555181), so I approved the backout:
https://treeherder.mozilla.org/#/jobs?repo=autoland&group_state=expanded&fromchange=57b8410f0239f2756915d3ac62f152f3e70c213d&searchStr=windows%2C10%2Cx64%2Cquantumrender%2Cshippable%2Copt%2Cmochitests%2Ctest-windows10-64-shippable-qr%2Fopt-mochitest-browser-chrome-e10s-1%2Cm%28bc1%29&tochange=dfa76bace85c114f9a410785b289b4fa630a01cd

That's why the change made it into central and the issue got detected with delay.

Patches aren't supposed to mature in tree, so they should get into a state where they cause no issues either by getting the underlying issue fixed (e.g. with help of other developers) or disabling the test (with the okay from its triage owner). Else that will cost the time of other developers doing Try pushes. In this case the failing platform has webrender enabled and the error message mentions webrender related code not called during startup. Every WR dev will have to identify that it's not their fault.

Regarding the eventualneedinfoing of florian: Like comment 10 mentions, his account is currently disabled. That's why needinfoing is not possible. Why needinfoing him? Because he is the test author: https://hg.mozilla.org/mozilla-central/log/tip/browser/base/content/test/performance/browser_startup_syncIPC.js

Flags: needinfo?(ccoroiu)

It seems like we agree that this is an issue with the test. Backing a patch out of central for increasing the frequency rate of an existing intermittent instead of investigating in the intermittent bug does not seem like a good policy to me and it's the first time I've seen it done. It seems like someone already made the decision to allow it on autoland (despite being aware of the failures) and I'm not sure if a frequent intermittent only on Windows10 QR is such a drastic case that it warrants overruling this decision.

Else that will cost the time of other developers doing Try pushes. In this case the failing platform has webrender enabled and the error message mentions webrender related code not called during startup. Every WR dev will have to identify that it's not their fault.

That's when you're weighing the cost to the developers of this patch against WR developers, but none of them should be fixing the test. Renaming the intermittent to "perma" and needinfo'ing the author is common practice AFAIU and it's unlikely to confuse anyone who has some experience with try.

Anyway, I'm sorry for starting a discussion in this bug. It's not the right place to question these decisions and I should have done so in a more constructive way. I understand that it's hard keeping the tree green and I appreciate your good work. I'll reach out to florian to get this resolved.

Thanks!

Flags: needinfo?(carolina.jimenez.g)

Florian, can you check why browser/base/content/test/performance/browser_startup_syncIPC.js started permafailing here? Thanks.

Flags: needinfo?(aryx.bugmail) → needinfo?(florian)

Florian has put up a fix in bug 1555181 and I'll land them both in succession now. The patch in bug 1555181 should fix/allowlist all failures that are known to us, but if you see any new failures of that patch on autoland for this patch, please feel free to back out again.

Thank you!

Flags: needinfo?(florian)
Pushed by jhofmann@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/898704612c74 Render real certificate information in about:certificate.r=johannh
Status: REOPENED → RESOLVED
Closed: 5 years ago5 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 70
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: