Bug 12686 – REGRESSION: Bloglines.com Feeds tab cannot expand folders in TOT

• Status: RESOLVED
• Resolution: FIXED
• Component: WebCore Misc.

[Charles Ying]

Repro steps:

1. Go to URL, log into Bloglines
2. Create a folder with feeds inside it
3. Try to expand the folder by clicking on the + button to the left of the folder
4. The folder does not expand, and a message is logged: "(event handler):Null value" on the Terminal

WebKit TOT does not work, (screenshot attached)
Safari 2.0.4 (419.3) works fine (screenshot attached)
Firefox 2.0.0.1 (Mozilla/5.0 (Macintosh; U; Intel Mac OS X; en-US; rv:1.8.1.1) Gecko/20061204 Firefox/2.0.0.1) works fine.

[Charles Ying]

Created attachment 13020 [details]
Bloglines screenshot with Expected results (Stock Safari + Firefox)

[Charles Ying]

Created attachment 13021 [details]
Bloglines error results in TOT WebKit and console error message

[Maciej Stachowiak]

<rdar://problem/4990043>

[Yongjun Zhang]

Created attachment 13194 [details]
Reduced test case.

The reduced test case shows:

In Safar 2.0, 'typeof document.getElementById("123")' returns "undefined" while TOT returns "object" and firefox returns something else (not "object", maybe "function").

However, the real problem is TOT doesn't support 'instanceof' for DOM objects, and the line "wh instanceof Element" returns false as a result. I wonder if we need to make DOMNode::implementsHasInstance() return true.

[Yongjun Zhang]

Just a quick check, I enabled hasInstance call for DOM object by making DOMNode::implementsHasInstance() return true.  The site works fine now.

See also bug 10686

[Maciej Stachowiak]

I'd love to see a patch along these lines.

[Yongjun Zhang]

Created attachment 13306 [details]
patch for review

Enable hasInstance for DOMObject by making implementsHasInstance return true.

[Darin Adler]

Comment on attachment 13306 [details]
patch for review

I believe this patch is incorrect. The constructor objects should return true for implementsHasInstance, but actual DOM element instances should not.

Thus I think the implementsHasInstance function should be in the constructor class definition inside CodeGeneratorJS.pm. It can go right above the implementsConstruct function.

To minimize the total amount of code inside CodeGeneratorJS.pm, we might want to make a DOMConstructor base class to inherit from. That's not needed to fix this bug, but I do see a reasonable amount of code that could be moved out of the auto-generation script, including:

    1) the setPrototype call in the constructor
    2) the getValueProperty member function
    3) the implementsConstruct member function
    4) the implementsHasInstance member function

But I'm not sure it's worth it.

We use spaces for indenting, not tabs. The patch has a number of tabs in it.

[David Kilzer (:ddkilzer)]

Fix committed by Kevin McCullough in r19993.

http://trac.webkit.org/projects/webkit/changeset/19993