Bug 5631 – KWQKHTMLPart::attributedString ignores many tags

• Status: VERIFIED
• Resolution: FIXED
• Component: HTML Editing

[Alexey Proskuryakov]

Because of what seems to be incorrect if/else nesting, KWQKHTMLPart::attributedString doesn't handle ol, 
ul, td, th, hr, dd, dl, dt, pre, blockquote, div, p, tr, h1, h2, h3, h4, h5, h6 and img tags.

I do not know how to reproduce the problem with shipping WebKit or ToT, but I have tested the fix (for 
DIV elements) with the proposed implementation of attributedSubstringFromRange (bug 4680) and my 
input method.

[Alexey Proskuryakov]

Created attachment 4601 [details]
proposed patch

[Darin Adler]

Comment on attachment 4601 [details]
proposed patch

The concept in Tiger was to get out of the business of making attributed
strings. AppKit was supposed to handle that for us. The only reason this code
is still around is that it was needed on Panther.

If we are going to keep and enhance this code that's inside WebCore (which I'm
not sure is a good idea), then we need to change it so that the text part of it
comes from the text iterator, so the text part of the attributed string will
match the plain text in the plain text string.

None of that really tells us whether or not to take this particular patch,
which might be an OK incremental improvement.

[Evan Gross]

(In reply to comment #2)
Just to muddy the attributed string waters a little more - take a look at what's in KWQAccObject.mm. 
Yet another way of creating attributed strings. Granted, the names of the attributes are different for AX 
objects, but same basic idea.

So attributed strings are currently created in 3 different ways (and counting?).

Maybe time to take another look at the "secret" one that's in AppKit? Do we know *anything* about 
what it can and can't do?

Anyway, playing with this patch, I don't see that it solves the problem with ranges that end with a line 
break that come from converting an NSRange to a DOMRange. Not sure if that's because of bug 5610 (I 
think it must be), or something other problem in KWQKHTMLPart::attributedString.

[Alexey Proskuryakov]

However, this specific patch only fixes what seems to be a simple typo :)

Yes, there are still problems with ranges that end with a newline, but at least newlines at other positions  
work better with it.

[Alexey Proskuryakov]

Created attachment 4841 [details]
patch with ignore-whitespace

Same patch made with cvs diff -ub for easier reading

[Eric Seidel (no email)]

Comment on attachment 4601 [details]
proposed patch

This patch looks totaly sane to me.  Regardless of whether this code is
deprecated or not.

[Darin Adler]

Good point. I didn't look at the patch closely enough when I first reviewed it. Now I get it. Landing it today.