prevent default on clicks to avoid firefox crash #160
Merged
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
This fixes #139. Currently the demo page crashes in Firefox (without popovers enabled) if you click the "Click to toggle shadowed popover" button.
Looking through a crash log, we can see this is because Firefox tries to handle the popover using their popover logic, but the logic isn't guarded to check the feature flag, and it tries to get the element by its ID, which is empty. The empty id check fails for some reason, and we end up with a crash:
mTarget->postHandleEvent(aVisitor);
postHandleEvent
callsHandlePopoverTargetAction();
HandlePopoverTargetAction
isn't adequately guarded so callsGetEffectivePopoverTargetElement();
GetAttrAssociatedElement(nsGkAtoms::popovertarget);
docOrShadowRoot->GetElementById(valueAtom);
aElementId
is anAtom
, but it should be the empty atom, and it should hitMOZ_UNLIKELY(aElementId == nsGkAtoms::_empty)
and return anullptr
, but for some reason it doesn't (it's not the empty atom?)mIdentifierMap.GetEntry(aElementId)
So, this is definitely a bug in Firefox. Exceptions like this shouldn't happen, I've filed https://bugzilla.mozilla.org/show_bug.cgi?id=1868740 and I'll work on a patch to fix it.
In the meantime, however, we have an opportunity to fix it ourselves, because
postHandleEvent()
will only be called if the event's default wasn't prevented. Consequently, simply addingevent.preventDefault()
in our JS fixes this issue.Steps to test/reproduce
Show me
N/A