On Tue, Sep 06, 2011 at 10:20:38AM -0300, Jorge Arellano Cid wrote:
On Mon, Sep 05, 2011 at 09:39:57PM +0200, Johannes Hofmann wrote:
On Mon, Sep 05, 2011 at 12:41:55PM -0300, Jorge Arellano Cid wrote:
On Mon, Sep 05, 2011 at 12:11:24AM +0200, Johannes Hofmann wrote:
[...] I don't know this part of the code well enough to say whether it is safe to commit before the release or not.
Actually I would suggest to commit attached patch. I have to admit that I'm not 100% sure about the ui.cc change, but it seems better than just always passing true or false.
AFAIS, it looks more correct.
The form.cc part makes sense, as limitTextWidth in textblock.cc seems to matter only if layout->getUsesViewport () returns true, which is not the case for FltkFlatView in the FltkComplexButton case.
corvid, Jorge, what do you think?
It works in the testcases (even outside forms), so I'd commit an release with it.
FWIW, WRT complexity, if you resize the window with the patch, over the testcase, it is quite clear that the last phrase is not measured right.
Yes, there is an issue, but I don't think it's related to the ComplexButton stuff.
Sure, it's related to whitespace handling in the textblock, which needs a full review.
<table> <tr> <td> <hr/> <div> words words words words words words words words words words words words words words words words words words words words words words words words words words words words words words words words words words words words words words words words words words words words <span style="white-space:nowrap">And words in a span</span> </div> </td> </table>
looks similarily weird here, or am I missing something?
I'm using an even simpler testcase:
<table> <tr> <td> words words words words words words words words words words words words words words words words words words words words words words <span style="white-space:nowrap">And words in a span</span> <button></button> </td> </table>
BTW, with regard to the patch, I'd keep the comment in the code, something like:
+ /* BUG: should be "Textblock (prefs.limit_text_width);", - * It caused 100% CPU usage with <button> or <input type="image"> + * long ago and needs a review before re-enabling it. */ - page = new Textblock (prefs.limit_text_width); + page = new Textblock (false); page->setStyle (html->styleEngine->backgroundStyle ());
Committed with slightly different wording in the comment, as I don't think using prefs.limit_text_width is correct in this case. Cheers, Johannes