NEW 68333
[meta] TextIterator refactoring 
https://bugs.webkit.org/show_bug.cgi?id=68333
Summary [meta] TextIterator refactoring
Ryosuke Niwa
Reported 2011-09-18 21:02:52 PDT
We should cleanup TextIterator.
Attachments
Add attachment proposed patch, testcase, etc.
Alexey Proskuryakov
Comment 1 2011-09-19 10:12:48 PDT
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.
Ryosuke Niwa
Comment 2 2011-09-19 10:18:54 PDT
(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.
Alexey Proskuryakov
Comment 3 2011-09-19 10:32:49 PDT
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.
Ryosuke Niwa
Comment 4 2011-09-19 10:45:35 PDT
(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
Alexey Proskuryakov
Comment 5 2011-09-19 10:54:07 PDT
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?
Ryosuke Niwa
Comment 6 2011-09-19 10:59:27 PDT
(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.
Note You need to log in before you can comment on or make changes to this bug.