Closed Bug 1218456 Opened 4 years ago Closed Last month does not trigger if element is not in document DOM


(Core :: DOM: Core & HTML, defect, P3)




Webcompat Priority ?
Tracking Status
firefox70 --- fixed


(Reporter: AdamK117, Assigned: emilio)


(Blocks 2 open bugs)


(Whiteboard: [specification][type:bug])


(6 files)

What did you do?
1) var a = document.createElement("a");
2) a.href = "";
3); // Does nothing
3) document.body.appendChild(a);
4); // Works

What happened?
Only works if the HTMLAnchorElement is appended into the document DOM.

What should have happened?
Unsure of DOM spec, but my intuition is that HTMLAnchorElement should navigate to href when calling .click().

Is there anything else we should know?
Firefox 41.0.2

This issue can make constructing Blob URLs using HTMLAnchorElements (and their download attribute) annoying. 

Instead of being able to write: = "myfile.csv";;

What's required for firefox is: = "myfile.csv";
a.document.body.removeChild(a); works for non-DOM elements in Google Chrome 46.0.2490.80 (64b), if that's anything to go by.
Product: Mozilla Developer Network → Developer Documentation
This is not a documentation bug, but a Gecko bug report.
Component: General → DOM: Core & HTML
Product: Developer Documentation → Core
What does Edge do?

I'm very surprised about Chrome's behavior. But that seems to be what the spec says, at least in some cases.
var a = document.implementation.createHTMLDocument().createElement("a"); a.href ="";;
seems to segfault in the spec (no browsing context), or am I missing something.
Flags: needinfo?(annevk)
> or am I missing something.

The definition of "fully active" should indeed handle the case of a document with no browsing context, and doesn't..

For the rest of this, I could have sworn I'd seen a similar bug report in the past, but I'm not finding it.
Filed to get that cleared up.
Flags: needinfo?(annevk)
(Should be clear now.)
OK.  Spec just (8 hours ago) got clarified in to say that this should work on `<a>` only, but not other potentially-navigating elements.  There are various web platform tests:

Looks like we already had bug 1037610 tracking this, but this one has more information, so I'll dup forward.
Ever confirmed: true
So most immediately, nsGenericHTMLElement::Click passes a null prescontext to event handling because it uses GetComposedDoc().  And then nsGenericHTMLElement::CheckHandleEventForAnchorsPreconditions bails out if no prescontext.

It would be pretty simple to change nsGenericHTMLElement::Click to get a prescontext from the owner doc, but this would not match the spec for click() on an <a> in a display:none iframe (not tested upstream, looks like), and would possibly change the behavior of click() on other HTML elements.

So my temptation is to remove that prescontext check from CheckHandleEventForAnchorsPreconditions (we have XXX comments about doing that already) and make the bits that follow work even without a prescontext.
Duplicate of this bug: 1402294
Priority: -- → P3

This bit me when fixing bug 1440537.

Flags: needinfo?(emilio)
See Also: → 1440537
See Also: → 1547524

So I thought this would be mostly straight-forward, and I had a patch that almost fixes it.

Mostly this, but beware of the TODOs which could make it more tricky:

The only think that's blocking me from fixing this is basically that we do the navigation off the DOMActivate event (rather than click), and the fact that we handle that event via the pres shell stuff:

Other browsers don't seem to dispatch DOMActivate when clicking on links. I see code dispatching those events for more nodes, but for links they don't go through HTMLElement::DefaultEventHandler:

Which is the only thing it could make it end there:

Also I'd note we barely have tests for that event.

So some paths forward in order of less to most difficulty / uncertainity (IMO):

  • Stop dispatching DOMActivate for link clicks and just do the navigation on click.
  • Maybe use plain EventDispatcher::Dispatch to dispatch DOMActivate. It's not clear to me why would it need the pres shell stuff (except we use to check stuff, but I think we should just use and co. instead of ESM / CurrentEventInfo.
  • Hoist HandleDOMEventWithTarget and to Document (not sure that's a great idea).

Masayuki, do you have any advice here? I'm happy to do any of the above (or none if you have better ideas!). Thanks in advance!

Flags: needinfo?(emilio) → needinfo?(masayuki)

Wow, really sorry for the long delay to reply. This was dropped from my queue accidentally.

According to the UI Events, we should keep firing DOMActivate event on anchor elements. I think that keeping it must be safer from point of view of backward compatibility. And if the link should cause navigation, the click event case of Element::PostHandleEventForLinks() should fall back to activate event case.

I don't think that it's a good way to use EventDispatcher::Dispatch() directly because looks like that it and handlers do not assume given pres context may be null.

And I think that smaug also should check the idea.

Flags: needinfo?(masayuki) → needinfo?(bugs)

It seems there may be versions of a library named imagem still in use which do not properly handle download buttons for generated images, as seen on

Webcompat Priority: --- → ?

Using plain EventDispatcher::Dispatch for DOMActivate should be fine, but need to then stop relying on prescontext in various places.

Flags: needinfo?(bugs)

Ok, I'll try to get time to work on this next week then. The prescontext dependency is mostly artificial, docshell is mostly what we need.

Assignee: nobody → emilio
Flags: needinfo?(emilio)

It's never overridden. Also chances are we should remove it and just use

Depends on D37404

Interfaces with just one implementation don't seem very useful.

Depends on D37405

I've checked that this doesn't change behavior in the test-case for bug 30178,
which is just an imagemap inside a link.

Overall it'd be weird if we needed this for area-inside-a but not for stuff like
nested links.

Depends on D37406

Flags: needinfo?(emilio)
Pushed by
Allow navigating when there's no pres context. r=smaug
Document::GetContainer shouldn't be virtual. r=smaug
Remove nsILinkHandler. r=smaug
Simplify CheckHandleEventForAnchorsPreconditions. r=smaug
Pushed by
Remove now-unneeded workarounds in cross-origin navigation tests. r=smaug
Pushed by
followup: Fix MinGW build bustage.
Backout by
Backed out 5 changesets for Crashtest failures on dom/l10n/tests/mochitest/dom_localization/test_overlay.html. CLOSED TREE

I'm not sure why the test structure is the way it is, but without this and with
the changes from this bug we start navigating away which means we try to reach, which is forbidden in automation since non-local connections are,
causing the failures that got me backed out in comment 25.

Backout by
Backed out changeset 60e865d21df5 for Crashtest failures on dom/l10n/tests/mochitest/dom_localization/test_overlay.html. CLOSED TREE
Pushed by
Prevent navigating away in test_overlay.html. r=zbraniecki
Pushed by
Document::GetContainer shouldn't be virtual. r=smaug
Pushed by
Allow navigating when there's no pres context. r=smaug
Remove nsILinkHandler. r=smaug
Simplify CheckHandleEventForAnchorsPreconditions. r=smaug
Pushed by
Remove now-unneeded workarounds in cross-origin navigation tests. r=smaug
Flags: needinfo?(emilio)
Pushed by
Port bug 1218456 - Remove include of nsILinkHandler.h from C-C. rs=bustage-fix DONTBUILD
Blocks: 1564980
You need to log in before you can comment on or make changes to this bug.