Bug 12686

Summary: REGRESSION: Bloglines.com Feeds tab cannot expand folders in TOT
Product: WebKit Reporter: Charles Ying <charles_ying>
Component: WebCore Misc.Assignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: kmccullough, KwhiteRight, yongjun.zhang
Priority: P1 Keywords: HasReduction, InRadar, Regression
Version: 420+   
Hardware: Mac (Intel)   
OS: OS X 10.4   
URL: http://www.bloglines.com/myblogs
Attachments:
Description Flags
Bloglines screenshot with Expected results (Stock Safari + Firefox)
none
Bloglines error results in TOT WebKit and console error message
none
Reduced test case.
none
patch for review darin: review-

Charles Ying
Reported 2007-02-07 16:35:23 PST
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.
Attachments
Bloglines screenshot with Expected results (Stock Safari + Firefox) (113.68 KB, image/jpeg)
2007-02-07 16:36 PST, Charles Ying
no flags
Bloglines error results in TOT WebKit and console error message (117.05 KB, image/jpeg)
2007-02-07 16:36 PST, Charles Ying
no flags
Reduced test case. (514 bytes, text/html)
2007-02-15 17:40 PST, Yongjun Zhang
no flags
patch for review (3.29 KB, patch)
2007-02-21 17:02 PST, Yongjun Zhang
darin: review-
Charles Ying
Comment 1 2007-02-07 16:36:15 PST
Created attachment 13020 [details] Bloglines screenshot with Expected results (Stock Safari + Firefox)
Charles Ying
Comment 2 2007-02-07 16:36:53 PST
Created attachment 13021 [details] Bloglines error results in TOT WebKit and console error message
Maciej Stachowiak
Comment 3 2007-02-10 19:12:49 PST
Yongjun Zhang
Comment 4 2007-02-15 17:40:44 PST
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
Comment 5 2007-02-15 17:47:25 PST
Just a quick check, I enabled hasInstance call for DOM object by making DOMNode::implementsHasInstance() return true. The site works fine now.
mitz
Comment 6 2007-02-17 04:13:10 PST
See also bug 10686
Maciej Stachowiak
Comment 7 2007-02-20 02:01:49 PST
I'd love to see a patch along these lines.
Yongjun Zhang
Comment 8 2007-02-21 17:02:24 PST
Created attachment 13306 [details] patch for review Enable hasInstance for DOMObject by making implementsHasInstance return true.
Darin Adler
Comment 9 2007-02-21 17:16:22 PST
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)
Comment 10 2007-03-06 18:50:36 PST
Fix committed by Kevin McCullough in r19993. http://trac.webkit.org/projects/webkit/changeset/19993
Note You need to log in before you can comment on or make changes to this bug.