Closed Bug 1397320 Opened 2 years ago Closed 2 years ago

Entering customize mode permanently hides EV labels for the current window

Categories

(Firefox :: Site Identity, defect)

defect
Not set

Tracking

()

VERIFIED FIXED
Firefox 57
Tracking Status
firefox-esr52 --- unaffected
firefox55 --- unaffected
firefox56 --- unaffected
firefox57 + verified
firefox58 --- verified

People

(Reporter: johannh, Assigned: dao)

References

Details

(Keywords: regression)

Attachments

(2 files)

[Tracking Requested - why for this release]:

Critical security UI regression.

STR:

- Go to a site with EV labels (such as bugzilla.mozilla.org)
- Enter customize mode and exit customize mode

The EV labels are permanently hidden, at least for me. See screenshot for what it looks like.

I have a very strong hunch that this was caused by bug 1396624 bug I can't prove it right now.
Flags: qe-verify+
Whiteboard: [photon-visual][triage] → [photon-structure][triage]
Flags: needinfo?(gijskruitbosch+bugs)
Regression window:
https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=76a7ead2334c65a6420038919047dce69d040166&tochange=5f580744fe7ae5350f6278ce70749a5696f8d4a5

Regresed by:
5f580744fe7a	Dão Gottwald — Bug 1396624 - Hide URL bar elements and the stop button with display:none instead of visibility:collapse to prevent the icons from being loaded needlessly. r=johannh
If this regressed as a result of bug 1396624, can one of you take it?
Flags: needinfo?(jhofmann)
Flags: needinfo?(gijskruitbosch+bugs)
Flags: needinfo?(dao+bmo)
Huh... That patch looked entirely unrelated at a first glance.
Assignee: nobody → dao+bmo
Status: NEW → ASSIGNED
Flags: needinfo?(dao+bmo)
Whiteboard: [photon-structure][triage]
(In reply to Dão Gottwald [::dao] from comment #3)
> Huh... That patch looked entirely unrelated at a first glance.

Turns out I overlooked multiple times that the selector contains #identity-icon-labels...
Flags: needinfo?(jhofmann)
Comment on attachment 8905646 [details]
Bug 1397320 - Use setAttribute to update identity block elements since they might be temporarily hidden with no binding attached.

https://reviewboard.mozilla.org/r/177434/#review182498

Amazing.

Where can I read up more about this particular gotcha of XUL bindings? I'd like to fully understand what's going on here to be able to flag it in future reviews.
Attachment #8905646 - Flags: review?(jhofmann) → review+
(In reply to Johann Hofmann [:johannh] from comment #7)
> Comment on attachment 8905646 [details]
> Bug 1397320 - Use setAttribute to update identity block elements since they
> might be temporarily hidden with no binding attached.
> 
> https://reviewboard.mozilla.org/r/177434/#review182498
> 
> Amazing.
> 
> Where can I read up more about this particular gotcha of XUL bindings? I'd
> like to fully understand what's going on here to be able to flag it in
> future reviews.

XBL was never documented properly so I'm not sure where I could point you to...
Pushed by dgottwald@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/e0ae495fe98d
Use setAttribute to update identity block elements since they might be temporarily hidden with no binding attached. r=johannh
https://hg.mozilla.org/mozilla-central/rev/e0ae495fe98d
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 57
Duplicate of this bug: 1398187
Build ID: 20170927100120
User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:58.0) Gecko/20100101 Firefox/58.0

Verified as fixed on Firefox Nightly 58.0a1 and Firefox 57.0b3 on Windows 10 x 64, Windows 7 x32, Mac OS X 10.12 and Ubuntu 16.04 x64.
Status: RESOLVED → VERIFIED
Flags: qe-verify+
You need to log in before you can comment on or make changes to this bug.