Closed Bug 1171517 Opened 5 years ago Closed 4 years ago

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

Categories

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

41 Branch
defect
Not set

Tracking

(firefox41 affected, firefox43 fixed)

RESOLVED FIXED
Firefox 43
Tracking Status
firefox41 --- affected
firefox43 --- fixed

People

(Reporter: jsantell, Assigned: jsantell)

References

(Blocks 2 open bugs)

Details

Attachments

(1 file)

It flips the pref but all subsequent recordings still have memory enabled
Some reason, hidden="true" doesn't do anything on the graph containers -- any idea?
Flags: needinfo?(vporof)
wat
Flags: needinfo?(vporof)
I'll take a look.
Summary: Turning off "enabling memory" doesn't actually turn it off → Turning off "enabling memory" doesn't hide memory graph
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.
Assignee: nobody → jsantell
Status: NEW → ASSIGNED
Summary: Turning off "enabling memory" doesn't hide memory graph → element.hidden properties on elements do not work when `display` property set
Comment on attachment 8651417 [details] [diff] [review]
1171517-fix-hidden.patch

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

omg
Attachment #8651417 - Flags: review?(vporof) → review+
https://hg.mozilla.org/mozilla-central/rev/cebf9a1b7e47
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → Firefox 43
> +    el.classList[isEnabled ? "remove" : "add"]("hidden");

FWIW, this can also be written |el.classList.toggle("hidden", !isEnabled);|.
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.