1171517 - element.hidden properties on elements do not work when `display` property set

Status: RESOLVED FIXED

(DevTools :: Performance Tools (Profiler/Timeline), defect)

Description

[Jordan Santell [:jsantell] [@jsantell]]

It flips the pref but all subsequent recordings still have memory enabled

Comment 1

[Jordan Santell [:jsantell] [@jsantell]]

Some reason, hidden="true" doesn't do anything on the graph containers -- any idea?

Comment 2

[Victor Porof [:vporof][:vp]]

wat

Comment 3

[Victor Porof [:vporof][:vp]]

I'll take a look.

Comment 4

[Jordan Santell [:jsantell] [@jsantell]]

Apparently we shouldn't be using `hidden` attribute for elements. From MDN[0]:

Changing the value of the CSS display property on an element with the hidden attribute overrides the behavior. For instance, an element styled display: flex will be displayed on screen regardless of the hidden attribute being present.

[0] https://developer.mozilla.org/en-US/docs/Web/HTML/Global_attributes/hidden

This is necessary right now for allocations, although this is like 4 layers deep in the rabbit hole.

Comment 5

[Jordan Santell [:jsantell] [@jsantell]]

Attachment: 1171517-fix-hidden.patch

https://treeherder.mozilla.org/#/jobs?repo=try&revision=44b630dde2c4

Comment 6

[Victor Porof [:vporof][:vp]]

Comment on attachment 8651417 [details] [diff] [review]
1171517-fix-hidden.patch

Review of attachment 8651417 [details] [diff] [review]:
-----------------------------------------------------------------

omg

Comment 7

[Pulsebot]

https://hg.mozilla.org/integration/fx-team/rev/cebf9a1b7e47

Comment 8

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/cebf9a1b7e47

Comment 9

[Simon Lindholm]

> +    el.classList[isEnabled ? "remove" : "add"]("hidden");

FWIW, this can also be written |el.classList.toggle("hidden", !isEnabled);|.