pages rendered with gtkmozembed stream API don't fire onload



Core Graveyard
Embedding: GTK Widget
13 years ago
6 years ago


(Reporter: Christian Persch (GNOME) (away; not receiving bug mail), Assigned: Marco Pesenti Gritti)


Firefox Tracking Flags

(Not tracked)



(2 attachments, 1 obsolete attachment)

Steps to reproduce:
0) Apply the attached patch to TestGtkEmbed, compile & run
1) Chose "Test stream" from File menu

Expected results:
Alert box.

Actual results:
No alert.
Created attachment 183195 [details] [diff] [review]
testcase as patch against TestGtkEmbed

Comment 2

12 years ago

This appears to be causing a11y problems for programs using the stream api.


Comment 3

12 years ago
The difference between "gtk_moz_embed_load_url" method and "gtk_moz_embed_open_stream" is "gtk_moz_embed_open_stream" creates new load group without observer.
See nsEmbedStream::OpenStream(nsIURI *aBaseURI, const nsACString& aContentType)
116   // create a new load group
117   rv = NS_NewLoadGroup(getter_AddRefs(mLoadGroup), nsnull);
118   if (NS_FAILED(rv))
119     return rv;

Instead, "gtk_moz_embed_load_url" uses nsDocShell.

This causes the yelp a11y bug (

I'm wondering if it is the same cause of not firing onload.
If so, how can we fix it?
> I'm wondering if it is the same cause of not firing onload.

Yes.  Onload is fired by the load group.

> If so, how can we fix it?

Perhaps EmbedStream should stop doing what it's doing and switch to nsIDocShell.loadStream?  I really don't like having these content-viewer-embedding stuff in EmbedStream.  Then again, this may not work well with AppendToStream, etc.

It could also do something akin to what nsDocShell::LoadStream does, but with its own channel.

Failing tall hat, it can get the right loadgroup off the docshell.

All this depends on what the function is _trying_ to do, of course.

Comment 5

8 years ago
Created attachment 386000 [details] [diff] [review]

This patch fixes the following:
1. Load events now fire.
2. An unlimited amount of data may be appended without blocking.
3. If openStream fails it can always be tried again.
4. The streamed document becomes part of the history.
I have also verified that if the content type includes a charset it will work as expected.
Attachment #386000 - Flags: superreview?(bzbarsky)
Attachment #386000 - Flags: review?(cbiesinger)

Comment 6

8 years ago
Bug 495055 will also be fixed by this patch.
Comment on attachment 386000 [details] [diff] [review]

Looks fine, assuming the file is supposed to be using tabs for indent.  If not, fix that, please.
Attachment #386000 - Flags: superreview?(bzbarsky) → superreview+

Comment 8

8 years ago
Boris, just to be sure, do you want me to convert the tabs to spaces?
I want you to be consistent with the existing indentation in the file.  If you're removing all existing longer-than-2 indentation, please do convert to spaces and add a corresponding modeline to the file.

Comment 10

8 years ago
Created attachment 386726 [details] [diff] [review]
Patch (same but with spaces instead of tabs and modeline)
Attachment #386000 - Attachment is obsolete: true
Attachment #386000 - Flags: review?(cbiesinger)

Comment 11

8 years ago
Boris, assuming the revised patch is OK can you push this for me please?
Last Resolved: 8 years ago
Resolution: --- → FIXED
Blocks: 495055

Comment 13

8 years ago
This caused a huge regression of yelp.

I'm not sure what is the root cause, since TestGtkEmbed is still working in mozilla-1.9.2.

Comment 14

8 years ago
just noticed, it was already fixed by
Component: Embedding: GTK Widget → Embedding: GTK Widget
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.