[webkit-reviews] review denied: [Bug 22333] Text wrapping algorithm for small screens (eg. Mobile screens) : [Attachment 25280] Patch for review

[bugzilla-daemon at webkit.org]

Antti Koivisto <koivisto at iki.fi> has denied maheshK
<mahesh.kulkarni at nokia.com>'s request for review:
Bug 22333: Text wrapping algorithm for small screens (eg. Mobile screens)
https://bugs.webkit.org/show_bug.cgi?id=22333

Attachment 25280: Patch for review
https://bugs.webkit.org/attachment.cgi?id=25280&action=review

------- Additional Comments from Antti Koivisto <koivisto at iki.fi>
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.