Bug 32250

Summary: Rename dom/ClassNames.{cpp,h}
Product: WebKit Reporter: Nate Chapin <japhet>
Component: WebCore Misc.Assignee: Nate Chapin <japhet>
Status: RESOLVED FIXED    
Severity: Normal CC: ap, michelangelo
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Attachments:
Description Flags
Rename ClassNames to SpaceSplitString darin: review+

Description Nate Chapin 2009-12-07 15:06:35 PST
ClassNames used to be named AtomicStringList, but was renamed (http://trac.webkit.org/changeset/28722) to reflect the fact that it was only being used for class names at the time.

Its behavior, however, matches what is required of HTML5 space separated lists (http://www.whatwg.org/specs/web-apps/current-work/#space-separated-tokens).  I used ClassNames for implementing <a rel="noreferrer"> support (https://bugs.webkit.org/show_bug.cgi?id=28986), with the understanding that ClassNames would need to be renamed again.

The only remaining question is what the new name should be.  I was thinking something like SpaceSplitString, but I could also revert it to AtomicStringList.  I'm not picky.
Comment 1 Alexey Proskuryakov 2009-12-07 15:11:25 PST
Probably best to e-mail webkit-dev for more people to see the question.
Comment 2 Nate Chapin 2009-12-08 13:41:58 PST
Created attachment 44482 [details]
Rename ClassNames to SpaceSplitString

I think I correctly modified all the build files, but is there a preferred way to confirm that?
Comment 3 Darin Adler 2009-12-08 15:43:35 PST
Comment on attachment 44482 [details]
Rename ClassNames to SpaceSplitString

> -__ZN7WebCore14ClassNamesData12createVectorEv
> +__ZN7WebCore14SpaceSplitStringData12createVectorEv

> -__ZN7WebCore14ClassNamesData11containsAllERS0_
> +__ZN7WebCore14SpaceSplitStringData11containsAllERS0_

These changes are incorrect. The "14" is the length of the string "ClassNamesData", so it should be "20" instead. But really this file should not be updated. There's no way for the "average person" to generate this. It has to be generated by running the framework in a test environment. Stephanie Lewis normally does it.

> Index: WebCore/html/HTMLAnchorElement.cpp
> ===================================================================
> --- WebCore/html/HTMLAnchorElement.cpp	(revision 51859)
> +++ WebCore/html/HTMLAnchorElement.cpp	(working copy)
> @@ -36,6 +36,7 @@
>  #include "Page.h"
>  #include "RenderImage.h"
>  #include "Settings.h"
> +#include "SpaceSplitString.h"

I don't understand how the name change triggered the need for an additional include.

I'm going to say r=me but I think both of those are errors. The build file changes look right to me.
Comment 4 Nate Chapin 2009-12-08 15:45:39 PST
(In reply to comment #3)
> (From update of attachment 44482 [details])
> > -__ZN7WebCore14ClassNamesData12createVectorEv
> > +__ZN7WebCore14SpaceSplitStringData12createVectorEv
> 
> > -__ZN7WebCore14ClassNamesData11containsAllERS0_
> > +__ZN7WebCore14SpaceSplitStringData11containsAllERS0_
> 
> These changes are incorrect. The "14" is the length of the string
> "ClassNamesData", so it should be "20" instead. But really this file should not
> be updated. There's no way for the "average person" to generate this. It has to
> be generated by running the framework in a test environment. Stephanie Lewis
> normally does it.
> 
> > Index: WebCore/html/HTMLAnchorElement.cpp
> > ===================================================================
> > --- WebCore/html/HTMLAnchorElement.cpp	(revision 51859)
> > +++ WebCore/html/HTMLAnchorElement.cpp	(working copy)
> > @@ -36,6 +36,7 @@
> >  #include "Page.h"
> >  #include "RenderImage.h"
> >  #include "Settings.h"
> > +#include "SpaceSplitString.h"
> 
> I don't understand how the name change triggered the need for an additional
> include.
> 
> I'm going to say r=me but I think both of those are errors. The build file
> changes look right to me.

That seems reasonable.  I'll change both prior to landing.  Thanks!
Comment 5 Nate Chapin 2009-12-09 10:07:45 PST
http://trac.webkit.org/changeset/51902