344830 - [FIX]referencing embed element with document.embeds['embedName'] or document.getElementById('embedId') in HTML document yields "invalid pointer" exception

Status: RESOLVED FIXED

(Core Graveyard :: Plug-ins, defect, P1)

Description

[Martin Honnen]

The test case at <http://home.arcor.de/martin.honnen/mozillaBugs/SVG/svgDocumentAccess1.html> is a HTML document with an embed element referencing an SVG document.
Firefox 2.0 beta (Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.1b1) Gecko/20060714 BonEcho/2.0b1) loads that SVG fine but the first attempt to script the SVG from the HTML document (by clicking the checkbox below the SVG) fails with 

Error: uncaught exception: [Exception... "Invalid pointer"  nsresult: "0x80004003 (NS_ERROR_INVALID_POINTER)"  location: "JS frame :: http://home.arcor.de/martin.honnen/mozillaBugs/SVG/svgDocumentAccess1.html :: toggleVisibility :: line 9"  data: no]

Line 9 does nothing but 
  var embed = document.embeds[svgName];

Further clicks to the checkbox do not result in further script errors and indeed then the script does what it is supposed to do, toggle the visibility of the SVG group g element.

This bug is restricted to the 1.8.1 Gecko/Firefox 2.0 branch.

The test case is reduced to the minimum but the problem was reported in the Mozilla SVG newsgroup by Andreas Neumann for real SVG applications.

The bug also shows itself with other HTML/SVG embed test pages such as this of James Watt: <http://jwatt.org/svg/demos/scripting-across-embed.html>:

Error: uncaught exception: [Exception... "Invalid pointer arg 1 [nsIDOMHTMLDocument.getElementById]"  nsresult: "0x80004003 (NS_ERROR_INVALID_POINTER)"  location: "JS frame :: http://jwatt.org/svg/demos/scripting-across-embed.html :: init :: line 25"  data: no]

So it seems accessing the embed element object the first time, whether with document.embeds or with document.getElementById, causes an error.

Comment 1

[Boris Zbarsky [:bzbarsky]]

This worksforme with a 1.8 branch firefox Linux build from end of June.  Is this a regression since then?  Or is this Windows-only?

It'd be nice to have testcases attached to the bug...

Comment 2

[Martin Honnen]

(In reply to comment #1)
> This worksforme with a 1.8 branch firefox Linux build from end of June.  Is
> this a regression since then?  Or is this Windows-only?

I can only test on Windows but Andreas in <http://groups.google.com/group/mozilla.dev.tech.svg/browse_frm/thread/896606300ec15e8f/38352cdc424b5261?hl=en#38352cdc424b5261> says he sees the problem on the Mac too.

Comment 3

[Martin Honnen]

(In reply to comment #1)
> This worksforme with a 1.8 branch firefox Linux build from end of June.  Is
> this a regression since then?  Or is this Windows-only?

Test case works for me on Windows with an older nightly Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.1a3) Gecko/20060625 BonEcho/2.0a3.

Test case shows the bug for me on Windows with Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.1b1) Gecko/20060708 BonEcho/2.0b1.

Comment 4

[Martin Honnen]

Attachment: SVG document needed for test case

Comment 5

[Martin Honnen]

Attachment: test case

Comment 6

[Boris Zbarsky [:bzbarsky]]

OK, sounds like a regression in the near past on the branch... We should try to narrow the range, I guess.

Comment 7

[Boris Zbarsky [:bzbarsky]]

This is fallout from the JS 1.7 landing, which started propagating the return value of PostCreate out.  Unfortunately, nsObjectFrame::GetPluginInstance will throw if it has no owner, instead of just returning null, so we end up throwing out of GetNewOrUsed...  The fact that a second time around succeeds is probably an XPConnect bug.

Comment 8

[Boris Zbarsky [:bzbarsky]]

Attachment: This seems to fix it

I _think_ this is safe, based on code audit...

Comment 9

[Christian :Biesinger (don't email me, ping me on IRC)]

Comment on attachment 229394 [details] [diff] [review]
This seems to fix it

hm... I guess http://lxr.mozilla.org/seamonkey/source/content/base/src/nsObjectLoadingContent.cpp#559 will not hit this case?

seems safe either way, though, so r=biesi.

Comment 10

[Christian :Biesinger (don't email me, ping me on IRC)]

though...

(In reply to comment #7)
> This is fallout from the JS 1.7 landing, which started propagating the return
> value of PostCreate out.

maybe PostCreate should instead be changed?

Comment 11

[Boris Zbarsky [:bzbarsky]]

> will not hit this case?

I _think_ it shouldn't, but I checked all the callers in any case...

> maybe PostCreate should instead be changed?

That's the other option -- special-casing this rv in PostCreate. But that assumes that it can't happen in any other way...

Comment 12

[Christian :Biesinger (don't email me, ping me on IRC)]

it could map any failure from GetPluginInstance to NS_OK

Comment 13

[Boris Zbarsky [:bzbarsky]]

I think that would be wrong.  For example, throwing OOM when it happens would be the right course of action instead of blindly doing the wrong thing...

Comment 14

[Christian :Biesinger (don't email me, ping me on IRC)]

hmm, ok, good point.

Comment 15

[Boris Zbarsky [:bzbarsky]]

Filed bug 344873 on nuking the wrapper if PostCreate fails so that things like this will throw consistently, not just the first time.

Comment 16

[Johnny Stenback (:jst)]

Comment on attachment 229394 [details] [diff] [review]
This seems to fix it

sr=jst

Comment 17

[Boris Zbarsky [:bzbarsky]]

Fixed on trunk

Comment 18

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Comment on attachment 229394 [details] [diff] [review]
This seems to fix it

a=dbaron on behalf of drivers.  Please check in to MOZILLA_1_8_BRANCH and mark
fixed1.8.1 once you have.

Comment 19

[Boris Zbarsky [:bzbarsky]]

Fixed on branch.

Comment 20

[Nickolay_Ponomarev]

*** Bug 344259 has been marked as a duplicate of this bug. ***

Comment 21

[Robert Sayre]

added to mochitest

RCS file: /cvsroot/mozilla/testing/mochitest/static/bug344830_testembed.svg,v
done
Checking in static/bug344830_testembed.svg;
/cvsroot/mozilla/testing/mochitest/static/bug344830_testembed.svg,v  <--  bug344830_testembed.svg
initial revision: 1.1
done

RCS file: /cvsroot/mozilla/testing/mochitest/tests/test_bug344830.html,v
done
Checking in tests/test_bug344830.html;
/cvsroot/mozilla/testing/mochitest/tests/test_bug344830.html,v  <--  test_bug344830.html
initial revision: 1.1
done