Bug 145775 - DOM elements appear ambiguous across frames (aliased?)
Summary: DOM elements appear ambiguous across frames (aliased?)
Status: RESOLVED FIXED
Alias: None
Product: konqueror
Classification: Applications
Component: khtml ecma (show other bugs)
Version: unspecified
Platform: RedHat Enterprise Linux Linux
: NOR normal
Target Milestone: ---
Assignee: Konqueror Developers
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2007-05-22 10:50 UTC by paul womack
Modified: 2008-01-19 18:43 UTC (History)
1 user (show)

See Also:
Latest Commit:
Version Fixed In:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description paul womack 2007-05-22 10:50:58 UTC
Version:            (using KDE KDE 3.5.6)
Installed from:    RedHat RPMs
OS:                Linux

With 2 sub pages in a frame set, there appears to be some VERY strange
aliasing of DOM elements.

There is a minimised example at http://www.papermule.co.uk/example/index.html

The code in frame TWO uses a fully disambiguated reference
to get to the "test" element in frame two.

parent.frames[1].document.getElementById("test");


Despite this, the code (both "make" and "show") appears
to be operating on COMPLETELY different elements
depending on wether the CALL to the code is from frame one
or frame two.

To demonstrate the bug, click on "show attribute" in both
frames. Both alert()s will show "undefined".

The click on "make attribute" button in frame 1.
"show attribute" in frame 1 now shows "10",
but "show attribute" in frame 2 still shows "undefined".

Now click on "make attribute" in frame 2, and
"show attribute" in frame 2 will show "20".

But show attribute in frame 1 STILL shows "10".

The expected behaviour is that setting the attribute should
work from EITHER frame, and that viewing should
also show the same output from either frame.
Comment 1 Maksim Orlovich 2007-05-22 17:40:04 UTC
By  'attribute' you mean an object property, right? Seems like we need to make the wrapper map global or such...
Comment 2 paul womack 2007-05-22 18:24:30 UTC
>> By  'attribute' you mean an object property, right?


I mean attributes as the term is used here...

http://www.w3.org/TR/1998/REC-DOM-Level-1-19981001/level-one-core.html

i.e. an attribute of a DOM element.

In javascript these can be accessed in multiple ways.

In the test, I use "element.name", which can be both read and written.

    BugBear
Comment 3 Maksim Orlovich 2007-05-22 20:58:14 UTC
Not true, actually. IE permits you to set attributes like that (which is non-standard!), and last I checked Mozilla doesn't. We actually do it weirdly and have only read access, which we should probably consider removing...
Comment 4 paul womack 2007-05-23 10:05:59 UTC
I don't want to argue (too much...) here, but the test works successfully on the following browsers:

SeaMonkey (Linix) 1.0.8
Safari (Mac) 2.0.4
Firefox (Linux) 1.5.0.8
IE 6 (XP) 6.0.2800.1106
IE 7 (XP) 7.0.5730.11

We also tried other attributes (out original app used a custom attribute)

Everything behaves "as expected" except for konqueror.

   BugBear
Comment 5 Maksim Orlovich 2007-05-23 16:17:13 UTC
Well, it doesn't disagree with the testcase working/not working --- the point is that if you tried to go getAttribute('name') in gecko after setting it as .name IIRC it would not work. But that's a side note...
Comment 6 paul womack 2007-05-23 16:31:58 UTC
OK, you're losing me here slightly. Accessing attributes is supported by all browsers, (including Konqueror) either via getAttribute and setAttribute, OR by assigning from/to the javascript objects. These are 2 variant syntaxes that express (AFAIK) the same thing.

If you don't me writing to the "name" attribute, I can assure you the behaviour is just the same with a custom attribute.

(The example I put up was simplified down from a rather larger context,and we were indeed using custom attributes).

This is NOT about the behaviour of a particular aspect, rather aspects accessed via the javascript object syntax in general (in Konqueror)

  BugBear
Comment 7 Maksim Orlovich 2007-05-23 16:42:19 UTC
Try the following testcase:
<script>
function test()
{
    document.body.name = 'foo';
    alert(document.body.getAttribute('name'));
}

</script>
<body onload="test()">
Comment 8 paul womack 2007-05-23 16:56:59 UTC
yep, tried that. Did as you said.

This test (well away from such matters) shows that the javascript object syntax, and setAttribute work in separate spaces (which I wasn't expecting)

<script>
function test()
{
    document.body.blah = 'foo';
    document.body.setAttribute( 'blah', 'foo2');
    alert("++" + document.body.blah + "--" + document.body.getAttribute('blah') + "++");
}

</script>
<body onload="test()">

However, in the test posted, the fault appears (IMHO) to be that the return value of parent.frames[1].document.getElementById("test"); varies with calling scope, thus allowing TWO separate values of the name property (accessed via the javascript object syntax) to co-exist.

I can put up a test of the same phenomona with a custom property if you wish.

   BugBear
Comment 9 Maksim Orlovich 2007-05-23 17:04:54 UTC
Yeah, pretty much, except IE actually unifies the two, and we have partial emulation of that which is kinda odd... But as you said, it's all irrelevant to this testcase..

The situation with your bug is as following --- and you pretty much got it right --- in order to permit stuff like this to work, we have a table that maps DOM objects to corresponding JS objects. However, the table is per interpreter/frame so the two call above get separate JS objects representing the same DOM objects. It seems like we need to share the table, which isn't hard except I'd have to double-check with a few people to make sure it doesn't screw anything up XSS-wise.

And thanks again for reporting this... I suspect some of the reports about websites are caused by this.
Comment 10 Allan Sandfeld 2007-06-13 13:25:48 UTC
One easy solution would be to complete the IE syntax, since attributes are already global. 

Personally I think it is Firefox that is being really odd by sharing properties globally.
Comment 11 Maksim Orlovich 2007-06-13 17:30:25 UTC
It wouldn't work completely since JS properties don't have to be strings (e.g. can be functions or objects)
Comment 12 Allan Sandfeld 2007-06-15 15:45:03 UTC
But could you still make the properties part the DOM, so it is stored with the object instead of in the semi-global table?
Comment 13 Maksim Orlovich 2007-06-15 16:35:47 UTC
Well, you would have to store a pointer to the wrapper object in every node, or be able to store arbitrary JS objects along attributes. (well, and then do a mark-and-sweep through the document, but it's sort of the right thing)

It'd be easiest to just make the wrapper map global --- the only reason I didn't do it is that I am not 100% certain of the XSS implications.
Comment 14 Maksim Orlovich 2008-01-19 18:43:46 UTC
SVN commit 763528 by orlovich:

Properly coalesce wrappers accross documents. This requires having a global
map for hashconsing along with the previous local wrapper maps
used for memory management. (We also have to be careful to insert into
local maps when reusing from global ones).

BUG: 145775

 M  +4 -1      kjs_binding.cpp  
 M  +26 -10    kjs_binding.h  


WebSVN link: http://websvn.kde.org/?view=rev&revision=763528