Last Comment Bug 709759 - Browser API: Stop page loading
: Browser API: Stop page loading
Status: RESOLVED FIXED
: dev-doc-complete
Product: Core
Classification: Components
Component: DOM (show other bugs)
: Trunk
: All All
: -- normal (vote)
: mozilla17
Assigned To: Dale Harvey (:daleharvey)
:
Mentors:
Depends on: 741717
Blocks: browser-api
  Show dependency treegraph
 
Reported: 2011-12-12 04:56 PST by Ben Francis [:benfrancis]
Modified: 2015-11-12 13:14 PST (History)
13 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Bug 709759 - Add stop() to mozbrowser API. (8.17 KB, patch)
2012-07-15 20:16 PDT, Dale Harvey (:daleharvey)
justin.lebar+bug: review-
Details | Diff | Review
Bug 709759 - Add stop() to mozbrowser API. r=jlebar (8.32 KB, patch)
2012-07-18 07:17 PDT, Dale Harvey (:daleharvey)
justin.lebar+bug: review+
Details | Diff | Review
Bug 709759 - Add stop() to mozbrowser API. r=jlebar (8.32 KB, patch)
2012-07-18 07:33 PDT, Dale Harvey (:daleharvey)
justin.lebar+bug: checkin+
Details | Diff | Review

Description Ben Francis [:benfrancis] 2011-12-12 04:56:29 PST
In order to allow a browser web app to have a stop button, add a stop() method to contentWindow.location which stops a page loading and can be called from the parent window, cross-origin.

As requested in Gaia https://github.com/andreasgal/gaia/issues/181
Comment 1 Justin Lebar (not reading bugmail) 2011-12-12 05:07:43 PST
I think the approach we're taking may be to add something like

  window.takeAction("stop");

because that's much easier to make available cross-origin.
Comment 2 Ben Francis [:benfrancis] 2011-12-12 05:15:03 PST
Just a thought, does that mean that if we wanted the equivalent for refreshing a page, we would have both:

window.location.reload()

and

window.takeAction("reload")

where you call a different method depending on what permissions you have...

This might be easier to implement in gecko, but as an API that we want to standardise, is this duplication a good thing?
Comment 3 Kyle Huey [:khuey] (khuey@mozilla.com) (Away until 6/13) 2011-12-12 05:18:24 PST
window.location already has weird stuff to work cross-origin, what's the harm in sticking another property on it?
Comment 4 Justin Lebar (not reading bugmail) 2011-12-12 05:24:13 PST
> This might be easier to implement in gecko, but as an API that we want to standardise, is this 
> duplication a good thing?

I'm not sure.  Standardizing "allow pages to violate same-origin protections" might be hard, whereas this API is more self-contained.

> window.location already has weird stuff to work cross-origin, what's the harm in sticking another 
> property on it?

More weird stuff is not necessarily good, for one thing.

But it's more of a general question, about more than window.location.  browser-as-webapp needs to read and write lots of properties in addition to the location: favicon URL, history, document title...  It also needs to register event listeners for things like locationchange.  So our choices are to

* drop same-origin protections altogether.   This is non-trivial.
* drop same-origin protections piecemeal.  This is probably harder.
* expose an API like this.  This is much simpler, although the API isn't as nice.

See also bug 708176, https://wiki.mozilla.org/WebAPI/EmbeddedBrowserAPI
Comment 5 Chris Jones [:cjones] inactive; ni?/f?/r? if you need me 2011-12-12 09:23:19 PST
Keep in mind that the browser app can't get ahold of a child <iframe>'s |window| in general, because it will live in another process.
Comment 6 Dale Harvey (:daleharvey) 2012-07-15 20:16:36 PDT
Created attachment 642469 [details] [diff] [review]
Bug 709759 - Add stop() to mozbrowser API.

This patch is based on top of the patch @ 
https://bugzilla.mozilla.org/show_bug.cgi?id=741717
Comment 7 Justin Lebar (not reading bugmail) 2012-07-16 12:58:01 PDT
Comment on attachment 642469 [details] [diff] [review]
Bug 709759 - Add stop() to mozbrowser API.

Clever test!

> +  content.document.defaultView.setTimeout(function() {
> +    stopped = true;
> +    iframe.stop();
> +  }, 200);

Nit: This is equivalent to window.setTimeout (or just setTimeout()).

We don't usually allow non-zero setTimeout's in our tests; they tend to invite randomorange.  But I think I understand your motivation here, and it's not so bad; you want to make sure the load is actually blocked on the image before canceling it, right?

Perhaps you could insert a <script>alert('finish');</script> after the <img> tag in the child.  Listen to mozbrowsershowmodaldialog and then call stop (perhaps off a timeout).

r- on the test only.
Comment 8 Dale Harvey (:daleharvey) 2012-07-18 07:17:57 PDT
Created attachment 643366 [details] [diff] [review]
Bug 709759 - Add stop() to mozbrowser API. r=jlebar

After discussing various ways to test this on irc, this came out as the best way, added comments to explain the test.
Comment 9 Justin Lebar (not reading bugmail) 2012-07-18 07:24:55 PDT
Comment on attachment 643366 [details] [diff] [review]
Bug 709759 - Add stop() to mozbrowser API. r=jlebar

r=me, but do you mind if I nit on the comment a bit?

> +// The img that is loaded will never be returned and will block
> +// the page from loading, the timeout ensures that the page is
> +// actually blocked from loading, once stop is called the
> +// image load will be cancaelled and mozbrowserloadend should be called.

This is a comma splice.  There are various ways to fix it, but I'd replace the first comma with a period, and the second comma with ", and".

s/cancaelled/cancelled/

But my main complaint is the claim that the timeout "ensures" that the page is blocked on the image load.  In truth, the timeout doesn't ensure anything; it only /tries/ to ensure something.
Comment 10 Dale Harvey (:daleharvey) 2012-07-18 07:33:18 PDT
Created attachment 643369 [details] [diff] [review]
Bug 709759 - Add stop() to mozbrowser API. r=jlebar

Corrections and clarifications in the comments.
Comment 11 Justin Lebar (not reading bugmail) 2012-07-18 07:59:44 PDT
Comment on attachment 643369 [details] [diff] [review]
Bug 709759 - Add stop() to mozbrowser API. r=jlebar

http://hg.mozilla.org/integration/mozilla-inbound/rev/e8ca855356d1
Comment 12 Ed Morley [:emorley] 2012-07-19 07:35:40 PDT
https://hg.mozilla.org/mozilla-central/rev/e8ca855356d1
Comment 13 Chris Mills (Mozilla, MDN editor) [:cmills] 2015-11-12 13:14:53 PST
Documented:

https://developer.mozilla.org/en-US/docs/Web/API/HTMLIFrameElement/stop

Note You need to log in before you can comment on or make changes to this bug.