Bug 14053 - Autogenerate JS binding for Rect
Summary: Autogenerate JS binding for Rect
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: DOM (show other bugs)
Version: 523.x (Safari 3)
Hardware: Mac OS X 10.4
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks: 13779
  Show dependency treegraph
 
Reported: 2007-06-09 18:22 PDT by Sam Weinig
Modified: 2007-06-15 19:00 PDT (History)
0 users

See Also:


Attachments
patch (30.50 KB, patch)
2007-06-09 18:31 PDT, Sam Weinig
no flags Details | Formatted Diff | Diff
updated patch (55.47 KB, patch)
2007-06-15 10:08 PDT, Sam Weinig
darin: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Sam Weinig 2007-06-09 18:22:27 PDT
Patch forthcoming, includes cleanup.
Comment 1 Sam Weinig 2007-06-09 18:31:04 PDT
Created attachment 14919 [details]
patch
Comment 2 Darin Adler 2007-06-14 20:47:27 PDT
Comment on attachment 14919 [details]
patch

Why are there so many changes here that are unrelated to DOMRect? Could we have the cleanup in a separate patch?

Can't we just rename RectImpl to WebCore::Rect? Why not?
Comment 3 Sam Weinig 2007-06-15 09:48:29 PDT
Comment on attachment 14919 [details]
patch

Thanks for reviewing this Tim, but I am removing the review flag and will post a new patch that renames RectImpl to Rect to be consistent.  I will post the patch shortly.
Comment 4 Sam Weinig 2007-06-15 10:08:09 PDT
Created attachment 15059 [details]
updated patch

This updated patch changes RectImpl to Rect and fixes the name conflict in WebKit by using ::Rect for the Rect struct in the global namespace.
Comment 5 Darin Adler 2007-06-15 16:23:00 PDT
Comment on attachment 15059 [details]
updated patch

     dom/Attr.idl \
-    dom/CharacterData.idl \
     dom/CDATASection.idl \

Why?

+    //rgbColors.remove(rgbColor.handle());

What is it, and why is this commented out?

Otherwise, r=me
Comment 6 Sam Weinig 2007-06-15 18:30:03 PDT
(In reply to comment #5)
> (From update of attachment 15059 [details] [edit])
>      dom/Attr.idl \
> -    dom/CharacterData.idl \
>      dom/CDATASection.idl \
> 
> Why?

This was just some sorting that went bad, has been fixed.

> +    //rgbColors.remove(rgbColor.handle());
> 
> What is it, and why is this commented out?

Not sure what this was, but it was in the old code commented out so I moved it over to the new stuff.  My guess it was an old caching mechanism, but since we are not going to ever resurrect it I have now removed it.
Comment 7 Sam Weinig 2007-06-15 19:00:03 PDT
Landed in r23557.