The function decamelize() cannot decamelize all strings the right way, so some hard coded fix ups are applied. At this moment, there are two functions that applied two different sets of fixups. There should be a single point to apply the same fix ups, which ensures that the result of decamelizing a string is always the same.
Created attachment 204439 [details] Patch
Created attachment 205009 [details] Patch
I forgot to remove "No new tests (OOPS!)."
Since this patch changes several parts of the code and it's hard to follow how those changes will affect the code generated, Xan suggested me to compare all the files under "WebKitBuild/Release/DerivedSources/webkitdom/" in branch master and a branch with this patch applied. I wrote a script to do that (http://pastebin.com/T14kWZpF). The script revealed the following files were different: * WebKitBuild/Release/DerivedSources/webkitdom/WebKitDOMWebKitNamedFlow.cpp * WebKitBuild/Release/DerivedSources/webkitdom/WebKitDOMWebKitPoint.cpp * WebKitBuild/Release/DerivedSources/webkitdom/WebKitDOMHTMLIFrameElement.cpp * WebKitBuild/Release/DerivedSources/webkitdom/WebKitDOMXPathResult.cpp The differences are due to an impromper decamelization of the variable "$nick" in master. Take for instance the file WebKitDOMHTMLIFrameElement.cpp, you can see code like this: g_object_class_install_property(gobjectClass, PROP_ALIGN, g_param_spec_string("align", /* name */ "htmli_frame_element_align", /* short description */ "read-write gchar* HTMLIFrameElement.align", /* longer - could do with some extra doc stuff here */ "", /* default */ WEBKIT_PARAM_READWRITE)); The string "htmli_frame_element_align" (value of the variable $nick), is not properly decamelized, it should be "html_iframe_element_align". This patch solves that unnoticed error too \o/ Besides "htmli", there are also an improper decamelization of the strings (as value of variable $nick) : * "web_kit" should be "webkit" * "x_path" shouldb be "xpath"
Comment on attachment 205009 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=205009&action=review > Source/WebCore/bindings/scripts/CodeGeneratorGObject.pm:137 > + return FixUpDecamelize($s); So, now that FixUpDecamelize is only called here, maybe we can add the implementation here?
Created attachment 205912 [details] Patch
Comment on attachment 205912 [details] Patch Attachment 205912 [details] did not pass gtk-ews (gtk): Output: http://webkit-queues.appspot.com/results/1016363
Comment on attachment 205912 [details] Patch >Subversion Revision: 152267 >diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog >index f82d3f32bbc7d9dedd0656ab18ab746363fbceaa..e30525e640b5ab899858c707d8c692be4ac90fca 100644 >--- a/Source/WebCore/ChangeLog >+++ b/Source/WebCore/ChangeLog >@@ -1,3 +1,28 @@ >+2013-07-02 Diego Pino Garcia <dpino@igalia.com> >+ >+ [GTK] Merge decamelizations fix ups in the GObject DOM bindings code generator >+ https://bugs.webkit.org/show_bug.cgi?id=117543 >+ >+ Reviewed by NOBODY (OOPS!). >+ >+ Ensure that all the code that calls to decamelization() applies the >+ same fix ups. >+ >+ Now all functions that need to decamelize a string should simply >+ call to decamelize(). This function calls to FixUpDecamelize to >+ apply some fix ups. >+ >+ * bindings/scripts/CodeGeneratorGObject.pm: >+ (decamelize): decamelizes a string and applies fix ups >+ (FixUpDecamelize): applies a series of fix ups to a decamelized string >+ (GetParentGObjType): moved fix ups to FixUpDecamelize() >+ (GenerateProperties): simply call to decamelize >+ (GenerateHeader): simply call to decamelize >+ (GetGReturnMacro): simply call to decamelize >+ (GenerateFunction): simply call to decamelize >+ (GenerateCFile): simply call to decamelize >+ (GenerateEventTargetIface): simply call to decamelize >+ > 2013-07-01 Tim Horton <timothy_horton@apple.com> > > Maximum scroll position can be negative in some cases >diff --git a/Source/WebCore/bindings/scripts/CodeGeneratorGObject.pm b/Source/WebCore/bindings/scripts/CodeGeneratorGObject.pm >index a52d47cc09c1370c4f4bbe721e40d604464b1959..061b13a27373bd54519b337349184dd20d7c7147 100644 >--- a/Source/WebCore/bindings/scripts/CodeGeneratorGObject.pm >+++ b/Source/WebCore/bindings/scripts/CodeGeneratorGObject.pm >@@ -134,18 +134,19 @@ sub decamelize > $t .= $p3 ? $p1 ? "${p1}_$p2$p3" : "$p2$p3" : "$p1$p2"; > $t; > }ge; >- $s; >-} >- >-sub FixUpDecamelizedName { >- my $classname = shift; > >- # FIXME: try to merge this somehow with the fixes in ClassNameToGobjectType >- $classname =~ s/x_path/xpath/; >- $classname =~ s/web_kit/webkit/; >- $classname =~ s/htmli_frame/html_iframe/; >- >- return $classname; >+ # Some strings are not correctly decamelized, apply fix ups >+ for ($s) { >+ s/domcss/dom_css/; >+ s/domhtml/dom_html/; >+ s/domdom/dom_dom/; >+ s/domcdata/dom_cdata/; >+ s/domui/dom_ui/; >+ s/x_path/xpath/; >+ s/web_kit/webkit/; >+ s/htmli_frame/html_iframe/; >+ } >+ return $s; > } > > sub HumanReadableConditional { >@@ -163,28 +164,11 @@ sub HumanReadableConditional { > return join(' ', @humanReadable); > } > >-sub ClassNameToGObjectType { >- my $className = shift; >- my $CLASS_NAME = uc(decamelize($className)); >- # Fixup: with our prefix being 'WebKitDOM' decamelize can't get >- # WebKitDOMCSS and similar names right, so we have to fix it >- # manually. >- $CLASS_NAME =~ s/DOMCSS/DOM_CSS/; >- $CLASS_NAME =~ s/DOMHTML/DOM_HTML/; >- $CLASS_NAME =~ s/DOMDOM/DOM_DOM/; >- $CLASS_NAME =~ s/DOMCDATA/DOM_CDATA/; >- $CLASS_NAME =~ s/DOMX_PATH/DOM_XPATH/; >- $CLASS_NAME =~ s/DOM_WEB_KIT/DOM_WEBKIT/; >- $CLASS_NAME =~ s/DOMUI/DOM_UI/; >- $CLASS_NAME =~ s/HTMLI_FRAME/HTML_IFRAME/; >- return $CLASS_NAME; >-} >- > sub GetParentGObjType { > my $interface = shift; > > return "WEBKIT_TYPE_DOM_OBJECT" if @{$interface->parents} eq 0; >- return "WEBKIT_TYPE_DOM_" . ClassNameToGObjectType($interface->parents(0)); >+ return "WEBKIT_TYPE_DOM_" . uc(decamelize($interface->parents(0))); > } > > sub GetClassName { >@@ -604,8 +588,9 @@ EOF > sub GenerateProperties { > my ($object, $interfaceName, $interface) = @_; > >- my $clsCaps = substr(ClassNameToGObjectType($className), 12); >- my $lowerCaseIfaceName = "webkit_dom_" . (FixUpDecamelizedName(decamelize($interfaceName))); >+ my $decamelize = decamelize($interfaceName); >+ my $clsCaps = uc($decamelize); >+ my $lowerCaseIfaceName = "webkit_dom_$decamelize"; > my $parentImplClassName = GetParentImplClassName($interface); > > my $conditionGuardStart = ""; >@@ -832,9 +817,9 @@ EOF > > push(@hBodyPre, $implContent); > >- my $decamelize = FixUpDecamelizedName(decamelize($interfaceName)); >+ my $decamelize = decamelize($interfaceName); > my $clsCaps = uc($decamelize); >- my $lowerCaseIfaceName = "webkit_dom_" . ($decamelize); >+ my $lowerCaseIfaceName = "webkit_dom_$decamelize"; > > $implContent = << "EOF"; > #define WEBKIT_TYPE_DOM_${clsCaps} (${lowerCaseIfaceName}_get_type()) >@@ -867,7 +852,7 @@ sub GetGReturnMacro { > if ($paramIDLType eq "GError") { > $condition = "!$paramName || !*$paramName"; > } elsif (IsGDOMClassType($paramIDLType)) { >- my $paramTypeCaps = uc(FixUpDecamelizedName(decamelize($paramIDLType))); >+ my $paramTypeCaps = uc(decamelize($paramIDLType)); > $condition = "WEBKIT_DOM_IS_${paramTypeCaps}($paramName)"; > if (ParamCanBeNull($functionName, $paramName)) { > $condition = "!$paramName || $condition"; >@@ -902,7 +887,7 @@ sub ParamCanBeNull { > sub GenerateFunction { > my ($object, $interfaceName, $function, $prefix, $parentNode) = @_; > >- my $decamelize = FixUpDecamelizedName(decamelize($interfaceName)); >+ my $decamelize = decamelize($interfaceName); > > if ($object eq "MediaQueryListListener") { > return; >@@ -1299,8 +1284,9 @@ sub GenerateCFile { > > my $implContent = ""; > >- my $clsCaps = uc(FixUpDecamelizedName(decamelize($interfaceName))); >- my $lowerCaseIfaceName = "webkit_dom_" . FixUpDecamelizedName(decamelize($interfaceName)); >+ my $decamelize = decamelize($interfaceName); >+ my $clsCaps = uc($decamelize); >+ my $lowerCaseIfaceName = "webkit_dom_$decamelize"; > my $parentImplClassName = GetParentImplClassName($interface); > my $baseClassName = GetBaseClass($parentImplClassName); > >@@ -1390,7 +1376,7 @@ sub GenerateEventTargetIface { > my $interface = shift; > > my $interfaceName = $interface->name; >- my $decamelize = FixUpDecamelizedName(decamelize($interfaceName)); >+ my $decamelize = decamelize($interfaceName); > my $conditionalString = $codeGenerator->GenerateConditionalString($interface); > my @conditionalWarn = GenerateConditionalWarning($interface); >
The last patch didn't apply because I used "//" to write a comment and Perl uses "#". I tried to modified the patch directly but don't know how to do it. I'm going to send the patch again.
Created attachment 205932 [details] Patch
Comment on attachment 205932 [details] Patch Thanks!
Comment on attachment 205932 [details] Patch Rejecting attachment 205932 [details] from commit-queue. Failed to run "['/Volumes/Data/EWS/WebKit/Tools/Scripts/webkit-patch', '--status-host=webkit-queues.appspot.com', '--bot-id=webkit-cq-01', 'apply-attachment', '--no-update', '--non-interactive', 205932, '--port=mac']" exit_code: 2 cwd: /Volumes/Data/EWS/WebKit Last 500 characters of output: lumes/Data/EWS/WebKit Parsed 2 diffs from patch file(s). patching file Source/WebCore/ChangeLog Hunk #1 succeeded at 1 with fuzz 3. patching file Source/WebCore/bindings/scripts/CodeGeneratorGObject.pm Hunk #2 FAILED at 164. 1 out of 8 hunks FAILED -- saving rejects to file Source/WebCore/bindings/scripts/CodeGeneratorGObject.pm.rej Failed to run "[u'/Volumes/Data/EWS/WebKit/Tools/Scripts/svn-apply', '--force', '--reviewer', u'Carlos Garcia Campos']" exit_code: 1 cwd: /Volumes/Data/EWS/WebKit Full output: http://webkit-queues.appspot.com/results/1130633
Created attachment 207078 [details] Patch
Comment on attachment 207078 [details] Patch Clearing flags on attachment: 207078 Committed r152898: <http://trac.webkit.org/changeset/152898>
All reviewed patches have been landed. Closing bug.