[Webkit-unassigned] [Bug 22333] Text wrapping algorithm for small screens (eg. Mobile screens)

[bugzilla-daemon at webkit.org]

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


koivisto at iki.fi changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #25280|review?(koivisto at iki.fi)    |review-
               Flag|                            |




------- Comment #10 from koivisto at iki.fi  2008-11-19 15:23 PDT -------
(From update of attachment 25280)
I wrote the original version of this code many years ago so I probably should
not be one to review it. Anyway

- I know what it does and why and how but it is probably not obvious to others.
You should include more comments to the code as well as to the change log
explaining it better.
- As Darin said, this has nothing to do with LOW_BANDWIDTH_DISPLAY so it should
not be behind that feature flag
- Coding style has many errors http://webkit.org/coding/coding-style.html
- It uses magic constants: 100, 30, 25. These should be at least be replaced
with named constants.
- frameWidth may be used uninitialized
- How did you test it? Are you sure this code works as expected with the
current bidi.cpp? The surrounding code has seen many changes yet this looks
very similar it used be.

All in all I'm not sure this code should checked in. It is pretty evil and I'm
not sure if anyone else is going to want to use it. It will break easily since
there are no buildbots building it. It might be better to apply this sort of
changes internally in organizations that want them. I don't see many benefits
to anyone in having it in public tree.


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