Bug 11686

Hi Eric, (In reply to comment #0) > WebKit draws Carto.net tabgroup example completely incorrectly (ff does fine) > > http://www.carto.net/papers/svg/gui/tabgroup/index.svg > > It looks like there is an issue measuring the text which is causing the tabs to > be so small. Then some text drawing issues, as well as some hit detection > issues. The hit detection issues might be related to the measuring. I looked into it and getBBox is wrong on the text element that gets created dynamically. It is basically an empty rect (0, 0, 0, 0), causing indeed measuring problems. It seems the render object is created but not layouted. Not sure how to handle this yet... Cheers, Rob.

> I looked into it and getBBox is wrong on the text element that gets created > dynamically. It is > basically an empty rect (0, 0, 0, 0), causing indeed measuring problems. It > seems the render object > is created but not layouted. Not sure how to handle this yet... I expect that getBBox just needs to force a layout, similar to how offsetTop works for HTML.

Created attachment 11719 [details] First attempt This could be a hack, but it works. Hyatt mentioned layoutIfNeeded being used a lot like this, but I couldn't really find it. Also I dislike the const_casts. I am wondering whether html text has the same problem as svg text, where the text is not layouted yet but the text dimensions are queried. Cheers, Rob.

Comment on attachment 11719 [details] First attempt You shouldn't try to force layout of a subtree like that, nor should the check be pushed down to the renderer. The way it's done in HTML is that the DOM method that needs layout information ensures that the document is laid out before querying the renderer. See Element::offsetTop() for example.

Created attachment 11721 [details] Improved patch Just what I was looking for, thnx Mitz! Cheers, Rob.

Created attachment 11722 [details] Also test paths and images Mitz was right, same problem can happen with images. This patch fixes it as well as does better testing. Cheers, Rob.

Comment on attachment 11722 [details] Also test paths and images I doubt that anything that doesn't apply to all SVGLocatables is correct and robust. For example, what if your element's renderer is a RenderSVGContainer? Can't it have text and image children? It could be my lack of familiarity with this code, but why would the straightforward solution of ensuring layout in SVGLocatable::getBBox not work? Minor style comments: please avoid "layouted" and use Sentence capitalization in comments.

Created attachment 11735 [details] Improved testcase + patch As Mitz pointed out, may as well make sure there is a layout done for all svg render objects. Cheers, Rob.

Created attachment 11744 [details] Improved testcase This patch has a fix in the comment. Furthermore it always performs the updateLayoutIgnorePendingStylesheets call, since potentially the element could have a parent that just had a display property changed from none. Cheers, Rob.

Created attachment 11745 [details] Improved patch Moved the call back into the if since render style updating means the renderer() must be there. Apart from that some rewording of the WebCore/ChangeLog. Cheers, Rob.

Comment on attachment 11745 [details] Improved patch r=me if you uppercase "svg" and change "are layout" to "are laid out" in the ChangeLog. I still don't like that sentence :-)

Landed by Rob in r18032.