Closed
Bug 1171517
Opened 9 years ago
Closed 9 years ago
element.hidden properties on elements do not work when `display` property set
Categories
(DevTools :: Performance Tools (Profiler/Timeline), defect)
Tracking
(firefox41 affected, firefox43 fixed)
RESOLVED
FIXED
Firefox 43
People
(Reporter: jsantell, Assigned: jsantell)
References
Details
Attachments
(1 file)
(deleted),
patch
|
vporof
:
review+
|
Details | Diff | Splinter Review |
It flips the pref but all subsequent recordings still have memory enabled
Assignee | ||
Updated•9 years ago
|
Blocks: perf-allocations
Assignee | ||
Comment 1•9 years ago
|
||
Some reason, hidden="true" doesn't do anything on the graph containers -- any idea?
Flags: needinfo?(vporof)
Comment 3•9 years ago
|
||
I'll take a look.
Assignee | ||
Updated•9 years ago
|
Summary: Turning off "enabling memory" doesn't actually turn it off → Turning off "enabling memory" doesn't hide memory graph
Assignee | ||
Comment 4•9 years ago
|
||
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
Blocks: 1197031, perf-tools-fx43
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
Assignee | ||
Comment 5•9 years ago
|
||
Attachment #8651417 -
Flags: review?(vporof)
Comment 6•9 years ago
|
||
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+
Comment 8•9 years ago
|
||
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
status-firefox43:
--- → fixed
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → Firefox 43
Comment 9•9 years ago
|
||
> + el.classList[isEnabled ? "remove" : "add"]("hidden");
FWIW, this can also be written |el.classList.toggle("hidden", !isEnabled);|.
Updated•6 years ago
|
Product: Firefox → DevTools
You need to log in
before you can comment on or make changes to this bug.
Description
•