User Agent: Mozilla/5.0 (Windows NT 5.1; rv:32.0) Gecko/20100101 Firefox/32.0 (Beta/Release) Build ID: 20140530030202 Steps to reproduce: 1. Open a page with at least two links 2. Click a link 3. Before the new page hasn't loaded, right click the second link 4. Wait until new page is loaded 5. Click "Open Link in New Tab" Actual results: The second link isn't opened. This error is produced: > TypeError: can't access dead object (nsContextMenu.js:849) Expected results: It should still work (like on Chrome). This behavior is very annoying when I find an interesting link and click it, but then I see another one I also want to visit. I right click it but, before I can open it in a new tab, the page is unloaded. These context menu entries don't work: - Open Link in New Tab - Open Link in New Window - Open Link in New Private Window - Bookmark This Link - Save Link As - Inspect Element (inspector opens but of course doesn't point at the rightclicked link) Only these do work: - Copy Link Location - Search <engine> for "<link content>"

With e10s enabled, this no longer reproduces. With it disabled, I can still reproduce intermittently. I've just been using bugzilla.mozilla.org as a testcase as it's slow enough to make it easy for me to get another link right-clicked before navigating away from the current page. Reporter, if you're willing, give a recent Nightly or Developer Edition release a try and see if it works OK for you as well.

Bug 1025568 - fix contextmenu actions across page loads, r?MattN

In non-e10s, at least in my testing, this breaks because by the time we get the context menu handler in content.js, doc.location is null, and so accessing doc.location.href throws, so the context menu data never gets set on the main window at all (leaving it null), which then breaks anything that accesses that data. We don't actually use docLocation unless you're using the "This frame" submenu, AFAICT, so it doesn't seem too bad if that's undefined - though I did also add a fallback to use doc.documentURI. I'm using undefined because it's what got set elsewhere (for frameUrl), so this seems most likely to be backwards compatible.

Comment on attachment 8678072 [details] MozReview Request: Bug 1025568 - fix contextmenu actions across page loads, r?MattN https://reviewboard.mozilla.org/r/23087/#review20841

https://reviewboard.mozilla.org/r/23087/#review20843 ::: browser/base/content/content.js:102 (Diff revision 1) > - let docLocation = doc.location.href; > + let docLocation = doc.location ? doc.location.href : (doc.documentURI || undefined); Are these ever out-of-sync mid-navigation? Any reason not to always use doc.documentURI with a comment about it being more accessible?

(In reply to Matthew N. [:MattN] from comment #5) > https://reviewboard.mozilla.org/r/23087/#review20843 > > ::: browser/base/content/content.js:102 > (Diff revision 1) > > - let docLocation = doc.location.href; > > + let docLocation = doc.location ? doc.location.href : (doc.documentURI || undefined); > > Are these ever out-of-sync mid-navigation? Any reason not to always use > doc.documentURI with a comment about it being more accessible? Hm, I don't know. I thought that documentURI didn't get updated for history state changes (ie history.pushState), but that doesn't seem to be true. Both include the hash. It seems that location.href is really the web navigation's URI, which is really nsDocShell's mCurrentURI, which is updated in a number of places, several of which note that it need not be identical to documentURI. See: https://dxr.mozilla.org/mozilla-central/source/docshell/base/nsDocShell.cpp#8594 I don't know how likely this is to make a material difference, and it's hard to tell from the immediate code. Boris, can you advise? (for context, this is data used by the context menu, mostly for opening frames in their own tabs/windows/whatever)

It seems like these are at least different for error pages, where location.href will be the erroring page, and document.documentURI will be about:blank . Doesn't seem like swapping these out all the time is sensible, but probably OK as a fallback value for these frame cases? Thoughts, Matt?

The biggest difference for the purposes you care about here is that doc.location.href is the URI of the docshell, which is not the same as the document URI if a different document has since been loaded. You're being saved by the fact that doc.location returns null if it's been navigated away from at the moment... but this is not necessarily the case per spec and may be subject to change.

(In reply to Boris Zbarsky [:bz] from comment #8) > The biggest difference for the purposes you care about here is that > doc.location.href is the URI of the docshell, which is not the same as the > document URI if a different document has since been loaded. > > You're being saved by the fact that doc.location returns null if it's been > navigated away from at the moment... but this is not necessarily the case > per spec and may be subject to change. This sounds like we should really just change to document.documentURI so that we always load the thing that was loaded at the time of the click. I don't think the error page case is really worth worrying about too much, though I'll wait for confirmation from MattN.

(In reply to :Gijs Kruitbosch (at OSCON, limited availability) from comment #7) > It seems like these are at least different for error pages, where > location.href will be the erroring page, and document.documentURI will be > about:blank . Doesn't seem like swapping these out all the time is sensible, > but probably OK as a fallback value for these frame cases? Thoughts, Matt? It's useful for the Page Info dialog to work on an error page so a user can remove cookies from the origin they were trying to load. I don't fully understand the consequences of changing this e.g. how often it will matter?

(In reply to Matthew N. [:MattN] from comment #10) > (In reply to :Gijs Kruitbosch (at OSCON, limited availability) from comment > #7) > > It seems like these are at least different for error pages, where > > location.href will be the erroring page, and document.documentURI will be > > about:blank . Doesn't seem like swapping these out all the time is sensible, > > but probably OK as a fallback value for these frame cases? Thoughts, Matt? > > It's useful for the Page Info dialog to work on an error page so a user can > remove cookies from the origin they were trying to load. > > I don't fully understand the consequences of changing this e.g. how often it > will matter? It seems like we could just fall back on null for now and I'll file a followup bug to discuss if/when we should use something else. The most-used items in the context menu aren't reliant on docLocation anyway, so we should at least stop breaking those.

Reproduced this issue on Windows 10x64 using Firefox 42 RC, build ID: 20151026170526. Confirming the fix using latest Nightly, build ID: 20151122030230. Tested on Windows 10 64bit, Mac OS X 10.11 and Ubuntu 14.04 x64.