Bug 11315 - Mutating document.constructor.prototype mutates Object.prototype
Summary: Mutating document.constructor.prototype mutates Object.prototype
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore JavaScript (show other bugs)
Version: 419.x
Hardware: Mac OS X 10.4
: P2 Normal
Assignee: Nobody
URL: http://bdash.net.nz/files/constructor...
Keywords:
Depends on:
Blocks:
 
Reported: 2006-10-16 08:50 PDT by Liam Clancy (metafeather)
Modified: 2007-06-25 23:43 PDT (History)
5 users (show)

See Also:


Attachments
patch (8.45 KB, patch)
2007-06-24 21:44 PDT, Sam Weinig
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Liam Clancy (metafeather) 2006-10-16 08:50:31 PDT
Not wanting to rehash discussions in the wider community about the use of Object.prototype to extend the native JS objects, I have found the following inconsitency between the MS Atlas Firefox and Safari compatibility layers that I do not think is intended, but on reviewing the JS code cannot see how this is happened unless there is a possible bug in Safari.

In AtlasCompat.js (see: http://atlas.asp.net/ScriptLibrary/Atlas/Release/AtlasCompat.js) the _loadSafariCompatLayer() function appears to add 'getElementById', 'attachEvent' and 'deleteEvent' functions to every native Object of the window rather than just the HTMLElement prototype Object that it intends.

To replicate:

1. Go to http://atlas.asp.net/atlastoolkit/Accordion/Accordion.aspx in Safari and Firefox (or other Atlas using site)

2. In each enter to following in the address bar to create a new Object and then iterate through its properties:

 javascript:var mytest = new Object();mytest.myprop = 'prop';for (var key in mytest){alert(key+' = '+typeof(mytest[key]))}

3. In Firefox there will be one alert with a value of 'myprop = prop'. In Safari there will be 4 alerts, including the extra function names (Tested in Safari 2.0.4 and Firefox 1.5.0.7)

I believe that this manipulation of Objects (in Safari only) is one of the reasons why developers have been having problems integrating custom code or other OS libraries (e.g. Prototype) with Atlas in Safari, and changing this will greatly simplify things.

This may be an MS Atlas JS bug so this is cross-posted to the ASP.NET AJAX Discussion forum here: http://forums.asp.net/1007/ShowForum.aspx (its being moderated).

However any work with the MS Atlas developers on their Safari Compatibility layer JS would be greatly appreciated.
Comment 1 Mark Rowe (bdash) 2006-10-16 14:39:11 PDT
I'm of the opinion that this bug is in the Atlas Safari compatibility layer.  Visiting the URL you provide in Safari produces the results that you describe.  If I tell Safari to use a Mozilla-derived user agent string, I get the expected behaviour of the single alert.  This suggests to me that the Atlas Safari "Compatibility layer" doesn't live up to its name.
Comment 2 Mark Rowe (bdash) 2006-10-16 14:41:03 PDT
Some further details for clarification:  I tested this in both WebKit 418.9 (latest released WebKit) and a development build of WebKit from this morning.  With the default Safari user agent, the problem is visible.  When spoofing as Firefox, the problem goes away.  Browser sniffing is by far the most likely cause.
Comment 3 Bertrand Le Roy 2006-10-17 18:19:22 PDT
(In reply to comment #2)
> Some further details for clarification:  I tested this in both WebKit 418.9
> (latest released WebKit) and a development build of WebKit from this morning. 
> With the default Safari user agent, the problem is visible.  When spoofing as
> Firefox, the problem goes away.  Browser sniffing is by far the most likely
> cause.

The Atlas compat layer has been completely redesigned so this problem should disappear with the next release, which is imminent.

Still, it seems a bit weird that extending the DOM element prototype would extend the Object prototype, as seems to be the case here... That might be a bug independently of Atlas and may be worth checking.
Comment 5 Mark Rowe (bdash) 2006-10-17 20:25:54 PDT
So I got bored and decided to reduce this issue to see what exactly was going wrong.  My initial assumption was that the Atlas had been doing something incorrect inside the Safari-specific portion of the JavaScript.  This code wasn't easily testable on other browsers, so I have reduced the issue.  The reduction is available from <http://bdash.net.nz/files/constructor-prototypes.html>.  The meat of the issue appears to be that document.constructor.prototype and document.createTextNode("").constructor.prototype both reference Object.prototype.  Adding properties to this results in all Object's gaining the new properties.  Firefox does not exhibit this behaviour on the reduction.
Comment 6 Bertrand Le Roy 2006-10-18 09:57:11 PDT
That explains it, thanks. We could actually have changed the document object directly instead of document.constructor.prototype as there is only one...
Personally, I wouldn't expect WebKit to fix that as long as document is of type Object, but...
Has the WebKit team considered doing what Firefox and Opera are doing, which is that elements are instances of HTMLElement, and document is an instance of HTMLDocument (both types deriving from Node)?
Comment 7 Alexey Proskuryakov 2007-06-22 05:26:07 PDT
Actually marking confirmed.
Comment 8 Geoffrey Garen 2007-06-22 11:29:59 PDT
I think there are two issues here:
1. HTMLDocument.prototype should be the document prototype, not the object prototype
2. HTMLDocument.prototype.__proto__ should == Node.prototype
Comment 9 Geoffrey Garen 2007-06-22 12:14:25 PDT
The problem seems to be that our DOM bindings don't generate the .constructor property correctly.

javascript:alert(document.constructor === window.Object) => true (should be false)
javascript:alert(document.constructor === window.HTMLDocument) => false (should be true)
Comment 10 Cameron McCormack (:heycam) 2007-06-22 17:46:50 PDT
(In reply to comment #8)
> I think there are two issues here:
> 1. HTMLDocument.prototype should be the document prototype, not the object
> prototype
> 2. HTMLDocument.prototype.__proto__ should == Node.prototype

Shouldn't HTMLDocument.prototype.__proto__ == Document.prototype?
Comment 11 Sam Weinig 2007-06-24 21:44:19 PDT
Created attachment 15213 [details]
patch
Comment 12 Maciej Stachowiak 2007-06-25 21:34:06 PDT
In general this looks good, but I think it would be good to cover a few more cases, maybe at least one HTML element, at least one SVG element, and a couple of things that aren't descended from Node at all.

r=me either way, but please consider improving the test.
Comment 13 Sam Weinig 2007-06-25 23:36:05 PDT
Landed in r23786.