Bug 4557 – rename unusual strcmp and strcasecmp for DOMString family

• Status: RESOLVED
• Resolution: FIXED
• Component: DOM

[Darin Adler]

I'm going to attach a patch with some DOMString and AtomicString improvements. Here are some of 
what's included:

    1) rename the unwieldy implementation() to impl()
    2) make more member functions inline
    3) remove the strange "strcmp" functions that return booleans
    4) replace the strange "strcasecmp" with "equalsIgnoringCase"
    5) add features so you can lowercase a string as it's created -- in the case of AtomicString, it can be 
done without copying the string

[Darin Adler]

Created attachment 3492 [details]
patch with the abovementioned changes

[Eric Seidel (no email)]

Just FYI... KDOM just went through and changed DOMString::implementation() to DOMString::handle()

[Darin Adler]

Given that the name of the class is DOMStringImpl I think impl() is a better name than handle(), but I could 
be convinced otherwise.

[Maciej Stachowiak]

impl() is a better name than handle(). It would be much nicer if these changes could be split into 
independent patches, especially the mechanical but huge implementation()/impl() converstion. I'm willing 
to rubber-stamp a global search-and-replace to do that by itself.

[Darin Adler]

Created attachment 3769 [details]
new patch, updated for today's TOT

[Darin Adler]

Created attachment 3770 [details]
refined a tiny bit more

[Darin Adler]

Created attachment 3771 [details]
uploading it again (fixed a mistake: sorry for the multiple copies)

[Darin Adler]

Created attachment 3786 [details]
patch that just fixes a function mistakenly renamed to qstring()

[John Sullivan]

Comment on attachment 3786 [details]
patch that just fixes a function mistakenly renamed to qstring()

What does removing XMLElementImpl::XMLElementImpl have to do with the rest of
this patch?

[Darin Adler]

Nothing.

That's just commented-out unused code that should be removed no matter what.

[Eric Seidel (no email)]

Comment on attachment 3786 [details]
patch that just fixes a function mistakenly renamed to qstring()

Simple patch.  Looks fine to me.

[Darin Adler]

Comment on attachment 3786 [details]
patch that just fixes a function mistakenly renamed to qstring()

I landed this patch, so I think I'll take the review flag off it.

[Darin Adler]

Created attachment 5361 [details]
eliminate strcmp, and replace strcasecmp with equalIgnoringCase

[Maciej Stachowiak]

Mostly looks good, though I'm not sure about adding new public constructors to DOMStringImpl without 
implementing them yet:

> +    DOMStringImpl(const QChar*, unsigned length, Lowercased);

[Maciej Stachowiak]

Comment on attachment 5361 [details]
eliminate strcmp, and replace strcasecmp with equalIgnoringCase

r=me but please consider the comment above. Also, perhaps ALLOC_QCHAR_VEC and
DELETE_QCHAR_VEC could be given normal looking function names now, as long as
you are renaming them.

[Darin Adler]

Making Maciej's requested changes and landing the patch.

[Lucas Forschler]

Mass moving XML DOM bugs to the "DOM" Component.