Currently if you want to refresh a page loaded inside a browser element/mozbrowser iframe you have to reset the src attribute which seems to work but is a little clunky. It would be nice to add a reload() method to the browser element, similar to the one <xul:browser> elements hav,e as a more explicit API for doing this. <xul:browser> also has a reloadWithFlags() method which allows you to do things like bypass the cache and force a full reload, we could consider adding something like this too.

Dale and I originally thought that we could just reset src to accomplish this. But of course that would truncate session history, which we don't want! So we do need this method.

Can't we use window.location.reload() or would that truncate session history too?

(In reply to Mounir Lamouri (:mounir) from comment #2) > Can't we use window.location.reload() or would that truncate session history > too? For an OOP iframe, iframe.contentWindow is null.

I have a strange problem in that this breaks the SecurityChange event (in process) 238 ERROR TEST-UNEXPECTED-FAIL | /tests/dom/browser-element/mochitest/test_browserElement_inproc_SecurityChange.html | undefined - got secure, expected insecure trying to investigate, I also likely need more tests but I cant think of what else to test.

>+ _recvReload: function(data) { >+ try { >+ let nsIWebNavigation = docShell.QueryInterface(Ci.nsIWebNavigation); >+ nsIWebNavigation.reload(nsIWebNavigation.LOAD_FLAGS_NONE); FYI, we'd send FLAGS_NONE for a "normal" reload, and BYPASS_PROXY | BYPASS_CACHE for a shift-reload (see browser/base/content/browser.js::BrowserReloadSkipCache()). On a phone, we don't have a shift-reload, so we might want normal reload to act like shift reload, or perhaps we should let the embedder ask for a normal/shift reload. But I think this is fine for now. >+ } catch(e) { >+ // Silently swallow errors; these can happen if a used cancels reload >+ } Can you elaborate on how you got an error here? >+/* Any copyright is dedicated to the public domain. >+ http://creativecommons.org/publicdomain/zero/1.0/ */ >+ >+// Bug 741717 - Test the reload ability of <iframe mozbrowser>. I think this test is sufficient if we don't add shift-reload semantics. I have no idea why this is breaking the securitychange test. :-/ Does the test still break if you don't reload here? Does the test still break if you put the SimpleTest.finish() behind a long-ish setTimeout?

Currently figuring out how this breaks security change notifications

Security (location) related bug is filed seperately https://bugzilla.mozilla.org/show_bug.cgi?id=741717 Will rebase and push this to try with in process tests disabled.

Justin I am having problems pushing this to try, https://github.com/mozilla/mozilla-central/commits/master and http://hg.mozilla.org/mozilla-central/ are diverged (I dont understand why) and git push-to-try is trying to push what looks like a few hundred commits Although I can import the patch to mercurial, when I try to qpush I get a bunch of conflicts, will figure out hg eventually but in the meantime if you could push that would help me, thanks, sorry for the bother.

>>+ } catch(e) { >>+ // Silently swallow errors; these can happen if a used cancels reload >>+ } > Can you elaborate on how you got an error here? I didnt get an error, but since the its specified that it may through, I thought it would be prudent to catch http://mxr.mozilla.org/mozilla-central/source/docshell/base/nsIWebNavigation.idl#241

> I am having problems pushing this to try, I did $ hg qpop -a $ hg pull -u $ hg qimport <(curl -L https://bug741717.bugzilla.mozilla.org/attachment.cgi?id=642453) $ hg qpush $ hg qref -e (add trychooser bits) $ hg push -f ssh://hg.mozilla.org > Although I can import the patch to mercurial, when I try to qpush I get a bunch of conflicts, Weird; I didn't get any conflicts. Maybe your hg tree is not at tip. That happens sometimes, particularly if you pushed some patches onto hg and then got conflicts. $ hg qpop -a; hg pull; hg up -C may fix it. > [git and hg] are diverged (I dont understand why) and git push-to-try is trying to push what looks > like a few hundred commits git push-to-try tries to push all the commits in |git qapplied|; it's not looking at hg at all for that information. It works by comparing HEAD to "upstream". But in the version in the master branch, "upstream" is always "origin/master". IOW, your problems may be solved if you |git fetch|. Alternatively, you can |git push-to-hg --tip| your patch (using an explicit rev list). You can't give git push-to-try an explicit rev list at the moment because it's hard for me to distinguish between a revision and trychooser params. But I should figure this out, because I want it too... Sorry, that's probably all TMI. Here's your push. :) https://tbpl.mozilla.org/?tree=Try&rev=7de45592fbef

> I didnt get an error, but since the its specified that it may through, I thought it would be prudent > to catch Ah...this is apparently the "do you want to re-send the POST data?" prompt (search for NS_BINDING_ABORTED in nsDocShell.cpp; there's only one occurrence). I wonder what happens if you try to refresh a POST page. My guess is that it is not pretty. :) Swallowing the exception seems right here, but we should probably file a follow-up on that dialog.

(btw, the bug number in the commit is wrong.) The try push is leaking in mochitest-other, but that is not your fault [1], so we're good here. [1] https://tbpl.mozilla.org/?tree=Mozilla-Inbound&rev=2220334ff2d0

Comment on attachment 642653 [details] [diff] [review] Bug 741717 - Add a reload method to browser elements. r=jlebar > + _reload: function(hardReload) { May the API Gods forgive us all. r=me with a few nits on the test: + var bodyCount = parseInt(data.json.data, 10); + switch (loadedEvents) { + case 1: + iframe.reload(); + break; + case 2: + ok(true, 'reload was triggered'); + lastLoadNum = bodyCount; + iframe.reload(true); + break; Are you intentionally not testing that reload() doesn't do a hard-reload? In this case, you're sending |Cache-Control: max-age=0|, so I would expect reload() /would/ do a hard-reload. But shouldn't we test it doing a soft one instead? + response.write('<html><body onload="opener.onChildLoad()" ' + + 'onunload="parseInt(\'0\')">' + What is this onunload handler for? If it's there for a good reason, could you please put a comment?

I was intentionally not testing soft reloads since its behavior wasnt predictable for me, it would sometimes load from cache and sometimes load the page, but as the comment below it looks like it may have been due to me resetting away some of my changes + response.write('<html><body onload="opener.onChildLoad()" ' + + 'onunload="parseInt(\'0\')">' + accidentally reverted some of my older changes Will reattach shortly

> Are you intentionally not testing that reload() doesn't do a hard-reload? > In this case, you're sending |Cache-Control: max-age=0|, so I would expect > reload() /would/ do a hard-reload. But shouldn't we test it doing a > soft one instead? I was intentionally not testing softreloads behaviour as I have never been able to get a reliable load from cache working, even currently using response.setHeader('Cache-Control', 'public, max-age=60'); a soft reload goes to the server, I would have argued that we didnt test softreload behaviour if I didnt find a bug in my patch where it was always hardreloading :) trying to figure out the caching now.

Now tests soft + hard reload I took the lets out of the try catch since we dont want to be silently failing if there is bugs there, just catching reload throws

Comment on attachment 642812 [details] [diff] [review] Bug 741717 - Add a reload method to browser elements. r=jlebar > +++ b/dom/browser-element/mochitest/file_bug741717.sjs Oh gosh, this looks horrible. I'm sorry! >+ let nsIWebNavigation = docShell.QueryInterface(Ci.nsIWebNavigation); Nit: Please call this webNav (or something), so it's clearly different from Ci.nsIWebNavigation. >+ let hardReloadFlags = nsIWebNavigation.LOAD_FLAGS_BYPASS_PROXY | >+ nsIWebNavigation.LOAD_FLAGS_BYPASS_CACHE; Nit: I think it would be a bit clearer if this was |const|. Also, you may want to indent the second line so nsIWebNavigation lines up on both lines. >+ let reloadFlags = data.json.hardReload ? hardReloadFlags : >+ nsIWebNavigation.LOAD_FLAGS_NONE; >+ try { >+ nsIWebNavigation.reload(reloadFlags); >+ } catch(e) { >+ // Silently swallow errors; these can happen if a used cancels reload >+ } >+ },

Documentation available here: https://developer.mozilla.org/en-US/docs/Web/API/HTMLIFrameElement.reload