[Webkit-unassigned] [Bug 27032] document.title does not replace or remove space characters

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=27032





--- Comment #4 from Christian Sejersen <christian.webkit at gmail.com>  2009-11-10 06:13:06 PDT ---
(In reply to comment #3)
> (From update of attachment 42702 [details])
> Thanks for taking this on!

No problem.

> 
> This new work seems to be a subset of the work already done in the
> DocumentLoader function canonicalizedTitle. I don't think Document::title
> should have its own separate copy of the logic. Some of what canonicalizedTitle
> does may be specific to display -- I'm thinking specifically of
> displayBufferModifiedByEncoding -- but most of it should be shared in common
> with Document::title.

It seems odd that the implementation in canonicalizedTitle is not using the
already existing methods in String to strip whitespace (e.g. the ones I used in
Document::title(). Also one of the comments in canonicalizedTitle is misleading
as it states that it will replace backslashes with currency symbols. That
doesn't happen until in displayBufferModifiedByEncoding.


> 
> I think that rather than adding new code for this, instead you should be moving
> some of the code from DocumentLoader to Document.

How about keeping the code I have for Document and then in DocumentLoader call
displayBufferModifiedByEncoding and remove the whitespace removal code?

> 
> A single title in the test case is really light coverage. I'd like to see a
> test case that sets all sorts of title strings to test the various edge cases
> and lots of characters other than just letters and the space character. The
> simplest way to write such a test would be to set and then get the document's
> title. I recommend a script-tests style test rather than writing your own mini
> test harness. See an example such as
> fast/Element/script-tests/element-traversal.js. The HTML driver file for the
> script test is made by typing make-script-test-wrappers.

I agree and thanks for the pointer. I will change and update the tests with
more coverage.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.