DocAccessibleParent::RecvShowEvent never sets lastParent
Categories
(Core :: Disability Access APIs, defect)
Tracking
()
People
(Reporter: Jamie, Assigned: Jamie)
References
(Blocks 1 open bug, Regression)
Details
(Keywords: regression)
Attachments
(1 file)
In bug 1779578, I changed the way a11y trees are serialised such that the parent id is included for each Accessible. In order to avoid a theoretical performance regression caused by repeatedly looking up the same parent, I added an optimisation to use the last parent if it is the same as the current parent. Unfortunately, it seems I never actually set the lastParent variable, so this optimisation was a no-op!
This is a micro-optimisation: it doesn't seem to make any observable difference. However, it seems silly to have effectively dead code and it's a very straightforward fix.
Test case for reference:
data:text/html,
<body role="application">
<div id="container" hidden></div>
<button id="insert">insert</button>
<button id="done">done</button>
<script>
insert.addEventListener("click", () => {
for (let i = 0; i < 500000; ++i) {
const p = document.createElement("p");
p.ariaLabel = i;
container.append(p);
}
container.hidden = false;
done.focus();
});
</script>
</body>
|
Assignee | |
Comment 1•1 year ago
|
||
In bug 1779578, I changed the way a11y trees are serialised such that the parent id is included for each Accessible.
When de-serialising in DocAccessibleParent::RecvShowEvent, in order to avoid a theoretical performance regression caused by repeatedly looking up the same parent, I added an optimisation to use the last parent if it is the same as the current parent.
Unfortunately, it seems I never actually set the lastParent and lastParentID variables, so this optimisation was a no-op!
This is a micro-optimisation: it doesn't seem to make any observable difference.
However, it seems silly to have effectively dead code and it's a very straightforward fix.
|
||
Comment 2•1 year ago
|
||
Set release status flags based on info from the regressing bug 1779578
|
||
Updated•1 year ago
|
|
||
Comment 4•1 year ago
|
||
| bugherder | ||
|
|
||
Comment 5•1 year ago
|
||
The patch landed in nightly and beta is affected.
:Jamie, is this bug important enough to require an uplift?
- If yes, please nominate the patch for beta approval.
- If no, please set
status-firefox128towontfix.
For more information, please visit BugBot documentation.
|
||
Updated•1 year ago
|
|
|
Assignee | |
Comment 6•1 year ago
|
||
This is a micro-optimisation: it doesn't seem to make any observable difference. Uplifting for no perceivable user benefit doesn't seem worthwhile.
Description
•