We should cleanup TextIterator.
Could you please describe the goals of this cleanup? From the bugs currently in blocking list, it's not clear to me what the problem is, and when this refactoring can be considered done.
(In reply to comment #1) > Could you please describe the goals of this cleanup? From the bugs currently in blocking list, it's not clear to me what the problem is, and when this refactoring can be considered done. The goal of this refactoring is to make TextIterator code more readable. By definition, it's unclear when the factoring is done. We have to use our judgement to decide when it's done.
What specifically is the problem with current code that you intend to solve? Can you describe a general direction for the changes? I haven't dealt with it too much, but it always seemed well written and understandable to me.
(In reply to comment #3) > What specifically is the problem with current code that you intend to solve? Can you describe a general direction for the changes? > > I haven't dealt with it too much, but it always seemed well written and understandable to me. If TextIterator and its variants are really well-written, I wouldn't expect bugs listed below to exist: https://bugs.webkit.org/show_bug.cgi?id=61110 https://bugs.webkit.org/show_bug.cgi?id=63334 https://bugs.webkit.org/show_bug.cgi?id=65296 https://bugs.webkit.org/show_bug.cgi?id=66086
Thanks. Could you please also give some insight into the first two questions? > What specifically is the [readability] problem with current code that you intend to solve? Can you describe a general direction for the changes?
(In reply to comment #5) > Thanks. Could you please also give some insight into the first two questions? Sure. > > What specifically is the [readability] problem with current code that you intend to solve? Can you describe a general direction for the changes? I think one of the issues is that TextIterator has a lot of member variables and so many different behaviors, so I'm hoping that we can simplify some of that. Also functions like TextIterator::advance, TextIterator::handleTextNode, TextIterator::handleTextBox should be broken down into smaller pieces with descriptive function names. Maybe bundling some member functions into a struct/class with a bunch of helper functions (like what we did with line layout code) might help. Finally, we should really consider sharing code between VisiblePosition, visible_units, TextIterator, etc... I think one of the issues we have now is that people working on rendering code don't necessary update all those files when they add new feature (first-letter is a good example) and some class/function gets out-of-date. Sharing code between those classes and centralizing it in one place will probably mitigate this issue to some extent.