Browser API: Stop page loading

RESOLVED FIXED in mozilla17

Status

()

Core
DOM
RESOLVED FIXED
5 years ago
2 years ago

People

(Reporter: benfrancis, Assigned: daleharvey)

Tracking

(Blocks: 1 bug, {dev-doc-complete})

Trunk
mozilla17
dev-doc-complete
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 2 obsolete attachments)

(Reporter)

Description

5 years ago
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
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.
(Reporter)

Comment 2

5 years ago
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?
window.location already has weird stuff to work cross-origin, what's the harm in sticking another property on it?
> 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
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.
Blocks: 715782
No longer blocks: 715782
(Reporter)

Updated

5 years ago
Summary: browser-as-webapp: Stop page loading → Browser API: Stop page loading
Keywords: dev-doc-needed
(Assignee)

Comment 6

5 years ago
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
Assignee: nobody → dale
Attachment #642469 - Flags: review?(justin.lebar+bug)
Depends on: 741717
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.
Attachment #642469 - Flags: review?(justin.lebar+bug) → review-
(Assignee)

Comment 8

5 years ago
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.
Attachment #642469 - Attachment is obsolete: true
Attachment #643366 - Flags: review?(justin.lebar+bug)
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.
Attachment #643366 - Flags: review?(justin.lebar+bug) → review+
(Assignee)

Comment 10

5 years ago
Created attachment 643369 [details] [diff] [review]
Bug 709759 - Add stop() to mozbrowser API. r=jlebar

Corrections and clarifications in the comments.
Attachment #643366 - Attachment is obsolete: true
Attachment #643369 - Flags: checkin?(justin.lebar+bug)
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
Attachment #643369 - Flags: checkin?(justin.lebar+bug) → checkin+
https://hg.mozilla.org/mozilla-central/rev/e8ca855356d1
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla17
Documented:

https://developer.mozilla.org/en-US/docs/Web/API/HTMLIFrameElement/stop
Keywords: dev-doc-needed → dev-doc-complete
You need to log in before you can comment on or make changes to this bug.