| Summary: | MathML preferred widths should not depend on layout information | ||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Ojan Vafai <ojan> | ||||||||||||||||||||||||
| Component: | MathML | Assignee: | Ojan Vafai <ojan> | ||||||||||||||||||||||||
| Status: | RESOLVED DUPLICATE | ||||||||||||||||||||||||||
| Severity: | Normal | CC: | abarth, alex, buildbot, cfleizach, darin, davidc, dbarton, dglazkov, eric, fred.wang, gyuyoung.kim, hyatt, inferno, jchaffraix, keishi, leviw, mitz, mrobinson, ojan.autocc, rakuco, rniwa, tony, webkit.review.bot | ||||||||||||||||||||||||
| Priority: | P2 | ||||||||||||||||||||||||||
| Version: | 528+ (Nightly build) | ||||||||||||||||||||||||||
| Hardware: | Unspecified | ||||||||||||||||||||||||||
| OS: | Unspecified | ||||||||||||||||||||||||||
| Bug Depends on: | 108023, 121416 | ||||||||||||||||||||||||||
| Bug Blocks: | |||||||||||||||||||||||||||
| Attachments: |
|
||||||||||||||||||||||||||
|
Description
Ojan Vafai
2013-01-18 18:48:53 PST
Created attachment 183592 [details]
Patch
Attachment 183592 [details] did not pass style-queue:
Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'LayoutTests/ChangeLog', u'LayoutTests/plat..." exit_code: 1
LayoutTests/platform/chromium-linux/mathml/presentation/sub-expected.png:0: Have to enable auto props in the subversion config file (/home/alancutter/.subversion/config "enable-auto-props = yes"). Have to set the svn:mime-type in the subversion config file (/home/alancutter/.subversion/config "*.png = svn:mime-type=image/png"). [image/png] [5]
Total errors found: 1 in 27 files
If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 183592 [details] Patch Attachment 183592 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/15975011 New failing tests: mathml/presentation/row.xhtml Why is this patch necessary? It makes MathML layout less precise, and doesn't fix any bugs that I know of. Is there a problem with the current code? This was discussed in bug 107197. I agree with David Carlisle - MathML layout is different, not wrong. I like his analogy to right-to-left writing. Also, the current MathML preferred widths code was duly tested and reviewed. Is there some bug or problem I'm not aware of? Thank you for looking into these MathML issues. I realize they're not your main interest, and I'm certainly not trying to gang up on you. But (again echoing Carlisle) I don't see why webkit can't lay out mathematics roughly like TeX or MathJax or Firefox or MathPlayer or other MathML renderers do. It's a consistent bottom-up algorithm, it's just not the same as old plain normal-flow HTML. Comment on attachment 183592 [details] Patch Attachment 183592 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/15964041 New failing tests: mathml/presentation/row.xhtml Comment on attachment 183592 [details] Patch Attachment 183592 [details] did not pass mac-ews (mac): Output: http://queues.webkit.org/results/15968021 New failing tests: mathml/presentation/fractions.xhtml mathml/presentation/sub.xhtml mathml/presentation/row.xhtml mathml/presentation/mo.xhtml mathml/presentation/mo-stretch.html mathml/presentation/tokenElements.xhtml mathml/presentation/attributes.xhtml mathml/presentation/row-alignment.xhtml mathml/presentation/fenced.xhtml mathml/presentation/style.xhtml mathml/presentation/subsup.xhtml mathml/presentation/fractions-vertical-alignment.xhtml mathml/presentation/over.xhtml mathml/presentation/fenced-mi.xhtml mathml/presentation/roots.xhtml mathml/presentation/underover.xhtml Comment on attachment 183592 [details] Patch Attachment 183592 [details] did not pass mac-ews (mac): Output: http://queues.webkit.org/results/15970035 New failing tests: mathml/presentation/fractions.xhtml mathml/presentation/sub.xhtml mathml/presentation/row.xhtml mathml/presentation/mo.xhtml mathml/presentation/mo-stretch.html mathml/presentation/tokenElements.xhtml mathml/presentation/attributes.xhtml mathml/presentation/row-alignment.xhtml mathml/presentation/fenced.xhtml mathml/presentation/style.xhtml mathml/presentation/subsup.xhtml mathml/presentation/fractions-vertical-alignment.xhtml mathml/presentation/over.xhtml mathml/presentation/fenced-mi.xhtml mathml/presentation/roots.xhtml mathml/presentation/underover.xhtml Comment on attachment 183592 [details] Patch Attachment 183592 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/15969038 New failing tests: mathml/presentation/row.xhtml Comment on attachment 183592 [details] Patch Attachment 183592 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://queues.webkit.org/results/15971085 New failing tests: mathml/presentation/fractions.xhtml mathml/presentation/sub.xhtml mathml/presentation/row.xhtml mathml/presentation/mo.xhtml mathml/presentation/mo-stretch.html mathml/presentation/attributes.xhtml mathml/presentation/row-alignment.xhtml mathml/presentation/fenced.xhtml mathml/presentation/tokenElements.xhtml mathml/presentation/style.xhtml mathml/presentation/subsup.xhtml mathml/presentation/fractions-vertical-alignment.xhtml mathml/presentation/over.xhtml mathml/presentation/fenced-mi.xhtml mathml/presentation/roots.xhtml mathml/presentation/underover.xhtml Created attachment 184021 [details]
Update to ToT and add TestExpectations
Comment on attachment 184021 [details]
Update to ToT and add TestExpectations
The parentheses seem to have changed weight. Was that the goal?
Attachment 184021 [details] did not pass style-queue:
Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'LayoutTests/ChangeLog', u'LayoutTests/plat..." exit_code: 1
LayoutTests/platform/chromium-linux/mathml/presentation/sub-expected.png:0: Have to enable auto props in the subversion config file (/home/alancutter/.subversion/config "enable-auto-props = yes"). Have to set the svn:mime-type in the subversion config file (/home/alancutter/.subversion/config "*.png = svn:mime-type=image/png"). [image/png] [5]
Total errors found: 1 in 32 files
If any of these errors are false positives, please file a bug against check-webkit-style.
(In reply to comment #4) > Why is this patch necessary? It makes MathML layout less precise, and doesn't fix any bugs that I know of. Is there a problem with the current code? This was discussed in bug 107197. I agree with David Carlisle - MathML layout is different, not wrong. I like his analogy to right-to-left writing. Also, the current MathML preferred widths code was duly tested and reviewed. Is there some bug or problem I'm not aware of? > > Thank you for looking into these MathML issues. I realize they're not your main interest, and I'm certainly not trying to gang up on you. But (again echoing Carlisle) I don't see why webkit can't lay out mathematics roughly like TeX or MathJax or Firefox or MathPlayer or other MathML renderers do. It's a consistent bottom-up algorithm, it's just not the same as old plain normal-flow HTML. As best I can tell, Firefox does not change the width of stretched operators based off their height. In the end, we should probably do whatever they're doing. They seem to get good looking results for our test cases at least. In the meantime, I don't want the MathML code imposing complexity on the rest of the rendering code. The main problem is that this violates constraints on what computePreferredLogicalWidths should do, which makes it hard for calling code to reason about what is going to happen, i.e., that computePreferredLogicalWidths will just compute the widths and not have other side effects. This leads to unintended consequences because of assumptions the calling code makes (e.g. security errors because the tree is different than expected). I don't think changing all the calling code to be safe in the presence of tree-modifications is worth the benefits of a more precise MathML layout. (In reply to comment #11) > (From update of attachment 184021 [details]) > The parentheses seem to have changed weight. Was that the goal? That's a nice benefit of the new approach, but is secondary to cleaning up computePreferredLogicalWidths. Once this patch goes in, I can add asserts that we never call setNeedsLayout (which includes render tree modifications) from within computePreferredLogicalWidths. (In reply to comment #13) > (In reply to comment #4) > > Why is this patch necessary? It makes MathML layout less precise, and doesn't fix any bugs that I know of. Is there a problem with the current code? This was discussed in bug 107197. I agree with David Carlisle - MathML layout is different, not wrong. I like his analogy to right-to-left writing. Also, the current MathML preferred widths code was duly tested and reviewed. Is there some bug or problem I'm not aware of? > > > > Thank you for looking into these MathML issues. I realize they're not your main interest, and I'm certainly not trying to gang up on you. But (again echoing Carlisle) I don't see why webkit can't lay out mathematics roughly like TeX or MathJax or Firefox or MathPlayer or other MathML renderers do. It's a consistent bottom-up algorithm, it's just not the same as old plain normal-flow HTML. > > As best I can tell, Firefox does not change the width of stretched operators based off their height. In the end, we should probably do whatever they're doing. They seem to get good looking results for our test cases at least. > > In the meantime, I don't want the MathML code imposing complexity on the rest of the rendering code. The main problem is that this violates constraints on what computePreferredLogicalWidths should do, which makes it hard for calling code to reason about what is going to happen, i.e., that computePreferredLogicalWidths will just compute the widths and not have other side effects. This leads to unintended consequences because of assumptions the calling code makes (e.g. security errors because the tree is different than expected). I don't think changing all the calling code to be safe in the presence of tree-modifications is worth the benefits of a more precise MathML layout. I still believe this patch is an unnecessary step backwards. The bug you mention is because of the render tree being modified during layout, not because of the accurate preferred width calculation. If you fix the render tree modification bug as you almost have (see bug 107197 comment 12) then the accurate preferred width causes no problem that I am aware of. Does Inferno or someone know of some bug that I don't? You are cleaning up some very old code here, which is good. But mainly you're saying that a delimiter should have the same width whether it is stretched or not, or regardless of how much it's stretched. This may not make our current rough MathML layout much worse but it prevents it from ever being good, as I say for no definite reason that I'm aware of. See https://eyeasme.com/Joe/MathML/MathML_browser_test.html for some TeX and Firefox images (from an old, perhaps better?, version of Firefox) in columns 1 and 2. The angle brackets in the Schwinger-Dyson equation and large parentheses in the nested matrices example, for instance, are wider than for normal size (unstretched) versions of these operators. Perhaps more important for web math, the integral at http://www.mathjax.org/ on MathJax's home page is wider than normal, as are the parentheses in its demo pages. For large matrices the effect would be more noticeable. A TeX expert could quantify this effect for us. Carlisle could probably give us many more examples. Obviously if this patch is necessary in the short term to fix major bugs, it's worth it. But I don't think it is, as we've seen no example of that, and the patch will need to be essentially backed out at some point if WebKit is ever to layout mathematics well. (In reply to comment #13) >. I don't think changing all the calling code to be safe in the presence of tree-modifications is worth the benefits of a more precise MathML layout. I'm sorry but you continue to underplay how much of a regression this is, it isn't making the layout "less precise" it is making it terribly bad. Clearly firefox like any other math renderer makes large characters wider than small ones: to do otherwise makes the expressions virtually unreadable in some cases. Dave Barton has again queried whether the security issue that you were fixing is really connected to this change to the character width calculation, I'd ask you to please look again at whether this is necessary. (In reply to comment #16) > (In reply to comment #13) > >. I don't think changing all the calling code to be safe in the presence of tree-modifications is worth the benefits of a more precise MathML layout. > > I'm sorry but you continue to underplay how much of a regression this is, it isn't making the layout "less precise" it is making it terribly bad. Have you seen the output from this patch? It's very different from the other patch. In fact, in many cases, the new output looks better to me. The only case that looks worse is the subscript on the integral. > Clearly firefox like any other math renderer makes large characters wider than small ones I don't see this in Firefox 18. Please give an example where this is true. (In reply to comment #15) > But mainly you're saying that a delimiter should have the same width whether it is stretched or not, or regardless of how much it's stretched. Correct. > This may not make our current rough MathML layout much worse but it prevents it from ever being good, as I say for no definite reason that I'm aware of. See https://eyeasme.com/Joe/MathML/MathML_browser_test.html for some TeX and Firefox images (from an old, perhaps better?, version of Firefox) in columns 1 and 2. The angle brackets in the Schwinger-Dyson equation and large parentheses in the nested matrices example, for instance, are wider than for normal size (unstretched) versions of these operators. I tested these in Firefox. If you increase the contents of the <mfenced>, e.g. in the Schwinger-Dyson equation, the angle brackets stay the same width, but grow taller. In the limit this looks bad, but it looks fine in all the examples on this page. It might be good enough in practice. > Obviously if this patch is necessary in the short term to fix major bugs, it's worth it. But I don't think it is, as we've seen no example of that, and the patch will need to be essentially backed out at some point if WebKit is ever to layout mathematics well. I am not convinced we'll ever need to make the width depend on the height. From a purist perspective, there are clearly cases where it's better, but I'm not sure we'll ever get to a point where that's the biggest issue with our mathml rendering and I'm not sure it matters much in practice. In the meantime, it doesn't make sense to have mathml impose a complexity cost on the rest of the codebase. Yes, this doesn't solve the updating the render-tree during layout issue, but it's a step in that direction (e.g. computePreferredLogicalWidths is called during layout). It looks like the rendering here is always better except maybe mathml/presentation/sub-expected? Created attachment 184079 [details]
tex source showing delimiter widths
Created attachment 184080 [details]
html/mathml source showing delimiter widths
Created attachment 184083 [details]
firefox rendering (FF21 nightly on windows)
Created attachment 184084 [details]
TeX rendering
(In reply to comment #17) > I don't see this in Firefox 18. Please give an example where this is true. > I attach example renderings in TeX and in Firefox. Frédéric would be able to confirm but it appears that in firefox the widths of the stretched delimiters are greater than the standard ones, although the width doesn't grow as much in TeX. In both cases of course once the stretching is being done by inserting vertical glyph segments the width no longer increases. (In reply to comment #17) > I am not convinced we'll ever need to make the width depend on the height. ... I'm not sure we'll ever get to a point where that's the biggest issue with our mathml rendering ... In that case I don't think WebKit's MathML rendering will ever be used in practice. Seriously, we can ask the folks who run the few big MathML sites if you don't believe me, or textbook publishers, software authors, etc. They'll use a slower, worse, non-HTML5 solution like MathJax or TeX, if it's the only way to get decent layout. (Or switch to a different solution altogether.) > Yes, this doesn't solve the updating the render-tree during layout issue, but it's a step in that direction (e.g. computePreferredLogicalWidths is called during layout). I think we're repeating here. computePreferredLogicalWidths currently calls some code that mutates the render tree during layout, which causes a bug. We could try to fix that bug, or just change that code to not mutate the tree, which you have almost done in another patch. If you land that, then computePreferredLogicalWidths is no longer calling code that mutates the tree. I don't see why you have to remove RenderMathMLBlock::computePreferredLogicalWidths then. If you do remove it, WebKit MathML layout will never be professional quality. David Carlisle is going to upload some simple examples, but really I'd suggest any book on TeX or math layout. Respectfully, I think you and Eric (in comment 18) are missing the point - you're just comparing these few small test results to WebKit's current very rough (bad) MathML layout, and maybe Firefox's. But to remove code now that allows tall delimiters to be wider than short ones now and in the future, when we don't even have a bug that it causes, just doesn't make sense to me. Maybe I just don't get "agile development". In bug 99614 I tried to discuss issues and get requirements analysis *before* choosing a design and writing code. I guess a security bug increases urgency, but this patch doesn't address the security bug. I think it comes down to, is good MathML rendering a requirement for WebKit or not? Sorry if I am repeating myself. I think I've made my points (several times), so I'll leave this issue to you guys now. Best wishes (seriously), Dave B. Related bug on adding the mentioned asserts bug 107613. You've convinced me that MathML operators need to have their width depend on the height of the row. In the end, when we get good MathML layout, this will need to be the case. Unfortunately, this is at fundamental odds with how HTML layout is designed. It makes MathML more like SVG in terms of the implications for implementation. As such, I think we need to move MathML to be more like SVGModelObject. Notably, it should not inherit from RenderBox. Whether MathML should be built on top of SVG or be it's entirely own Render* stack is unclear to me at the moment. So, given that. The question is what to do in the short-term. I have a new version of this patch that centers the operator. IMO, the new rendering is better in almost all cases than our current rendering looking at the equations on https://eyeasme.com/Joe/MathML/MathML_browser_test.html. There are some cases where it's minorly worse. Since the proper way to support MathML operators will look completely different from either the old code or the new code, this doesn't take us further away from the goal of having good MathML rendering. I'm inclined to move forward with this patch. Seem fair? Created attachment 184319 [details]
Patch
I'll upload some before and after shots. Attachment 184319 [details] did not pass style-queue:
Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'LayoutTests/ChangeLog', u'LayoutTests/platform/chromium-linux/mathml/presentation/fenced-expected.txt', u'LayoutTests/platform/chromium-linux/mathml/presentation/fenced-mi-expected.png', u'LayoutTests/platform/chromium-linux/mathml/presentation/fenced-mi-expected.txt', u'LayoutTests/platform/chromium-linux/mathml/presentation/mo-expected.txt', u'LayoutTests/platform/chromium-linux/mathml/presentation/mo-stretch-expected.png', u'LayoutTests/platform/chromium-linux/mathml/presentation/mo-stretch-expected.txt', u'LayoutTests/platform/chromium-linux/mathml/presentation/over-expected.txt', u'LayoutTests/platform/chromium-linux/mathml/presentation/row-alignment-expected.txt', u'LayoutTests/platform/chromium-linux/mathml/presentation/row-expected.png', u'LayoutTests/platform/chromium-linux/mathml/presentation/row-expected.txt', u'LayoutTests/platform/chromium-linux/mathml/presentation/sub-expected.png', u'LayoutTests/platform/chromium-linux/mathml/presentation/sub-expected.txt', u'LayoutTests/platform/chromium-linux/mathml/presentation/subsup-expected.txt', u'LayoutTests/platform/chromium/TestExpectations', u'LayoutTests/platform/efl/TestExpectations', u'LayoutTests/platform/gtk/TestExpectations', u'LayoutTests/platform/mac/TestExpectations', u'LayoutTests/platform/qt/TestExpectations', u'Source/WebCore/ChangeLog', u'Source/WebCore/mathml/MathMLTextElement.cpp', u'Source/WebCore/mathml/MathMLTextElement.h', u'Source/WebCore/rendering/mathml/RenderMathMLBlock.cpp', u'Source/WebCore/rendering/mathml/RenderMathMLBlock.h', u'Source/WebCore/rendering/mathml/RenderMathMLFenced.cpp', u'Source/WebCore/rendering/mathml/RenderMathMLOperator.cpp', u'Source/WebCore/rendering/mathml/RenderMathMLOperator.h', u'Source/WebCore/rendering/mathml/RenderMathMLRoot.cpp', u'Source/WebCore/rendering/mathml/RenderMathMLRoot.h', u'Source/WebCore/rendering/mathml/RenderMathMLRow.cpp', u'Source/WebCore/rendering/mathml/RenderMathMLRow.h']" exit_code: 1
LayoutTests/platform/chromium-linux/mathml/presentation/sub-expected.png:0: Have to enable auto props in the subversion config file (/home/alancutter/.subversion/config "enable-auto-props = yes"). Have to set the svn:mime-type in the subversion config file (/home/alancutter/.subversion/config "*.png = svn:mime-type=image/png"). [image/png] [5]
Total errors found: 1 in 25 files
If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 184321 [details]
Patch
Attachment 184321 [details] did not pass style-queue:
Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'LayoutTests/ChangeLog', u'LayoutTests/platform/chromium-linux/mathml/presentation/fenced-expected.txt', u'LayoutTests/platform/chromium-linux/mathml/presentation/fenced-mi-expected.png', u'LayoutTests/platform/chromium-linux/mathml/presentation/fenced-mi-expected.txt', u'LayoutTests/platform/chromium-linux/mathml/presentation/mo-expected.txt', u'LayoutTests/platform/chromium-linux/mathml/presentation/mo-stretch-expected.png', u'LayoutTests/platform/chromium-linux/mathml/presentation/mo-stretch-expected.txt', u'LayoutTests/platform/chromium-linux/mathml/presentation/over-expected.txt', u'LayoutTests/platform/chromium-linux/mathml/presentation/row-alignment-expected.txt', u'LayoutTests/platform/chromium-linux/mathml/presentation/row-expected.png', u'LayoutTests/platform/chromium-linux/mathml/presentation/row-expected.txt', u'LayoutTests/platform/chromium-linux/mathml/presentation/sub-expected.png', u'LayoutTests/platform/chromium-linux/mathml/presentation/sub-expected.txt', u'LayoutTests/platform/chromium-linux/mathml/presentation/subsup-expected.txt', u'LayoutTests/platform/chromium/TestExpectations', u'LayoutTests/platform/efl/TestExpectations', u'LayoutTests/platform/gtk/TestExpectations', u'LayoutTests/platform/mac/TestExpectations', u'LayoutTests/platform/qt/TestExpectations', u'Source/WebCore/ChangeLog', u'Source/WebCore/mathml/MathMLTextElement.cpp', u'Source/WebCore/mathml/MathMLTextElement.h', u'Source/WebCore/rendering/mathml/RenderMathMLBlock.cpp', u'Source/WebCore/rendering/mathml/RenderMathMLBlock.h', u'Source/WebCore/rendering/mathml/RenderMathMLFenced.cpp', u'Source/WebCore/rendering/mathml/RenderMathMLOperator.cpp', u'Source/WebCore/rendering/mathml/RenderMathMLOperator.h', u'Source/WebCore/rendering/mathml/RenderMathMLRoot.cpp', u'Source/WebCore/rendering/mathml/RenderMathMLRoot.h', u'Source/WebCore/rendering/mathml/RenderMathMLRow.cpp', u'Source/WebCore/rendering/mathml/RenderMathMLRow.h']" exit_code: 1
LayoutTests/platform/chromium-linux/mathml/presentation/sub-expected.png:0: Have to enable auto props in the subversion config file (/home/alancutter/.subversion/config "enable-auto-props = yes"). Have to set the svn:mime-type in the subversion config file (/home/alancutter/.subversion/config "*.png = svn:mime-type=image/png"). [image/png] [5]
Total errors found: 1 in 25 files
If any of these errors are false positives, please file a bug against check-webkit-style.
(In reply to comment #26) > You've convinced me that MathML operators need to have their width depend on the height of the row. In the end, when we get good MathML layout, this will need to be the case. Unfortunately, this is at fundamental odds with how HTML layout is designed. It makes MathML more like SVG in terms of the implications for implementation. As such, I think we need to move MathML to be more like SVGModelObject. Notably, it should not inherit from RenderBox. I disagree strongly with each of the last 4 sentences. The first sentence I addressed in bug 107613 comment 8 and elsewhere. For the second, e.g. SVG forbids the CSS box model, whereas MathML embraces it. For the third and fourth, it will be long indeed before webkit spends the developer resources on MathML that it has on SVG, and that this would require. Nor is it necessary, as the current approach has produced no bugs. MathML benefits greatly from inheriting from flexbox. Your plan to eventually move flexbox to not inherit from RenderBlock sounds great to me, but I'd stop there. Isn't the main point to RenderBox that it stores box model values for the CSS box model? MathML benefits from this, and other flexbox features. > Whether MathML should be built on top of SVG or be it's entirely own Render* stack is unclear to me at the moment. I would like to hear an SVG expert say whether putting MathML on top of it would be easy. I don't think it'd be easier than flexbox, because of the CSS box model. > So, given that. The question is what to do in the short-term. I have a new version of this patch that centers the operator. IMO, the new rendering is better in almost all cases than our current rendering looking at the equations on https://eyeasme.com/Joe/MathML/MathML_browser_test.html. There are some cases where it's minorly worse. > > Since the proper way to support MathML operators will look completely different from either the old code or the new code, this doesn't take us further away from the goal of having good MathML rendering. Since I disagree with the "rewrite MathML totally" assumption, I disagree with this conclusion. The MathML code is currently only about 2000 lines, not counting copyright notices. It needs some to be added, but still it's really small, and we can't get resources for working on that. A total SVG-size rewrite just isn't likely. Nor is it necessary. You're throwing away MathML's preferred width code when it isn't causing any problems. What's wrong with waiting until an actual bug comes up? > I'm inclined to move forward with this patch. > > Seem fair? (In reply to comment #26) > You've convinced me that MathML operators need to have their width depend on the height of the row. The suggested patch as far as I understand it still removes that calculation from webkit so is not really acceptable. Your comments about SVG confuse me as MathML is _far_ more like HTML than SVG in its rendering behaviour. Mathematics is _text_ : it flows with the paragraph and (should) take part in line breaking in a natural way. It has some special requirements just as embedding Arabic in English has some special requirements that were probably not met with initial rendering implementations but now are. It should be the same with MathML. I should note here that while we are saying "MathML" This is not some peculiar feature of the MathML language that could have been specified differently, it is a natural requirement of the text setting and would be an issue whatever format is used to mark up a bracket. The requirement that a large operator is wider than a small one is just so natural, and implemented in the existing code, in firefox and in every other Mathematical rendering system I have seen. It's completely baffling why one should have to have this conversation at all. If there are security issues in the actual code implementation that need to be fixed, then they should of course be flagged. SVG is essentially an image format, you never expect an SVG image to wrap over a line. and (mostly) SVG constructs are governed by specified coordinates rather than size naturally to their children like an html/css table or flexbox. Almost all MathML constructs take on a size given by their children. I think what we're trying to do here is to assert that no layout or render tree change happens inside computePreferredLogicalWidths and it never calls setNeedsLayout. Is there a way to achieve that without deleting these functions? Perhaps we need a separate re-layout phase for MathML to prepare for the calls to computePreferredLogicalWidths? The current implementation is not great because the rest of css-related render objects don't expect computePreferredLogicalWidths to modify render tree but at the same time, it seems inconceivable to make widths not dependent on height in MathML. Given that we need to figure out a way to implement MathML properly without having to modify render tree within computePreferredLogicalWidths. (In reply to comment #34) > I think what we're trying to do here is to assert that no layout or render tree change happens inside computePreferredLogicalWidths and it never calls setNeedsLayout. One way to do this would be for RenderMathMLBlock have a method like computeHeightWithoutLayout() that would return the height of the MathML block without running a layout. The code in computeHeightWithoutLayout() would be similar to what's in layout (get the height of each child, then call computeLogicalHeight), but it wouldn't need to mess with the layout bit or the render tree. (In reply to comment #34) > I think what we're trying to do here is to assert that no layout or render tree change happens inside computePreferredLogicalWidths and it never calls setNeedsLayout. > > Is there a way to achieve that without deleting these functions? Perhaps we need a separate re-layout phase for MathML to prepare for the calls to computePreferredLogicalWidths? > > The current implementation is not great because the rest of css-related render objects don't expect computePreferredLogicalWidths to modify render tree ... Can you say a bit more about that? I agree that at least for now, operator stretching should happen via scaling not stacking. So no render tree nodes would be created or destroyed. However, why is it bad to call setNeedsLayout on a descendent, and then do the layout (thus clearing the setNeedsLayout), all protected by a LayoutStateDisabler? (I'm asking an honest question, not a rhetorical one.) The bad bug I've seen, and security issues Ahbishek talked about at the meeting last April as I recall, have to do with render tree nodes being created and destroyed, not just having their metrics calculated early. The problem with a separate phase is that you need to know the heights of siblings, essentially calculating everything that you would to do an actual layout. > but at the same time, it seems inconceivable to make widths not dependent on height in MathML. Given that we need to figure out a way to implement MathML properly without having to modify render tree within computePreferredLogicalWidths. (In reply to comment #35) > (In reply to comment #34) > > I think what we're trying to do here is to assert that no layout or render tree change happens inside computePreferredLogicalWidths and it never calls setNeedsLayout. > > One way to do this would be for RenderMathMLBlock have a method like computeHeightWithoutLayout() that would return the height of the MathML block without running a layout. The code in computeHeightWithoutLayout() would be similar to what's in layout (get the height of each child, then call computeLogicalHeight), but it wouldn't need to mess with the layout bit or the render tree. I agree with this, except I'm asking why it's necessary. There's also maybe a performance problem since you'd be essentially laying out twice. With recursion, this would be 2^n perhaps, or maybe just an extra factor of O(n), where n is the tree depth. We could mostly eliminate that by caching this preferred logical height. Actually RenderMathMLBlock.h does that now. But what's really happening is bottom-up layout, not top-down, it seems to me. Why exactly is this a problem? In summary, if turning layout bits on and off is really a problem, this approach seems next best. It requires extra duplicate code and time (an extra pass). Should we wait to do it until a bug or person gives us a definite problem with the current implementation? (In reply to comment #36) > Can you say a bit more about that? I agree that at least for now, operator stretching should happen via scaling not stacking. So no render tree nodes would be created or destroyed. However, why is it bad to call setNeedsLayout on a descendent, and then do the layout (thus clearing the setNeedsLayout), all protected by a LayoutStateDisabler? (I'm asking an honest question, not a rhetorical one.) It's all about timing. There are certain phases of layout during which we don't want to modify the render tree, and there are certain invariants we want to enforce on some member functions of RenderObject in order to mitigate infinite recursion and ultimately security bugs. One of such invariant we'd like to have is to make computePreferredLogicalWidths a readonly operation. > The problem with a separate phase is that you need to know the heights of siblings, essentially calculating everything that you would to do an actual layout. Yeah, I agree this is a hard problem. > (In reply to comment #35) > > (In reply to comment #34) > > > I think what we're trying to do here is to assert that no layout or render tree change happens inside computePreferredLogicalWidths and it never calls setNeedsLayout. > > > > One way to do this would be for RenderMathMLBlock have a method like computeHeightWithoutLayout() that would return the height of the MathML block without running a layout. The code in computeHeightWithoutLayout() would be similar to what's in layout (get the height of each child, then call computeLogicalHeight), but it wouldn't need to mess with the layout bit or the render tree. > > I agree with this, except I'm asking why it's necessary. There's also maybe a performance problem since you'd be essentially laying out twice. With recursion, this would be 2^n perhaps, or maybe just an extra factor of O(n), where n is the tree depth. We could mostly eliminate that by caching this preferred logical height. Actually RenderMathMLBlock.h does that now. > > But what's really happening is bottom-up layout, not top-down, it seems to me. Why exactly is this a problem? Because of the rest of the rendering code assumes top-down layout. There are few places where we need to do bottom-up but those are rare exceptions at least in the HTML/CSS world. If we really need a bottom-up layout, we need to a major architectural change. My suggestion is to do two-pass layout since the performance wasn't the biggest problem in MathML the last time I checked. > In summary, if turning layout bits on and off is really a problem, this approach seems next best. It requires extra duplicate code and time (an extra pass). Should we wait to do it until a bug or person gives us a definite problem with the current implementation? We need to address this problem ASAP. The problem is that, as you pointed out, there are very few people working on MathML but there are dozens if not hundreds of engineers working on the core rendering engine. And MathML is preventing this major refactoring effort to tighten constraints on computePreferredLogicalWidths, imposing a significant cost on the project. It's great that we have a working MathML implementation and there are people working hard to improve it, but we can't let the core code suffer either. Sorry for all the comments/e-mail, but I'm coming around to the computeHeightWithoutLayout() suggestion, sort of. I think this should happen inside computePreferredLogicalWidths for RenderMathMLBlock elements, and the resulting preferred logical height should be cached for them, sort of like what happens now. The difference would be to avoid using layout to accomplish this, as you all suggest, if that is really bad. From an implementation point of view, preferred logical heights for MathML elements are just like preferred logical widths, calculated at the same time and in the same ways, pretty much (though there's only one preferred logical height, not a min and max). There are a few MathML-related points (simplifications) to make. First, possible line wrapping has to be ignored, or else height-affects-width-affects-height really is circular. So a bracket is probably only tall enough to contain a single-line expression. (The current code does this.) Second, it's very important for math layout to use the actual heights of glyphs, not the font size. (-webkit-line-box-contain mostly does this for us.) However, the font size might be good enough or even better for the height of adjacent brackets, i.e. this preferredLogicalHeight we're discussing. (It is important not to use the font ascent + descent, because math fonts like STIX and Cambria Math tend to have huge values for these, due to a few very tall operators.) Lastly, <mtext> and other "token" elements can contain arbitrary HTML, at least as far as parsing is concerned. The spec and related comments actually recommend handling arbitrary DOM content there, but indicate that perhaps only inline HTML is totally "legal". So I think we could use the font size as a preferredLogicalHeight there as well perhaps, without worrying about font size changes/etc. inside html inside <mtext> elements, so we wouldn't need to do a full layout or implement preferred logical height calculations for all elements, just RenderMathMLBlock ones. Does this sound right, both from an implementation and MathML point of view? One problem is we would need this preferred height calculation for tables, definitely, since that's the main use case for large delimiters, namely matrices. <mtable>, <mtr>, and <mtd> are rendered using RenderTable... classes, so they'd need some kind of computeHeightWithoutLayout() function. Perhaps in this case at least, we wouldn't add a separate m_preferredLogicalHeight, but we'd just use the current logicalHeight() storage? Is this what Tony's suggesting by calling computeLogicalHeight? (In reply to comment #37) > (In reply to comment #36) > > Can you say a bit more about that? I agree that at least for now, operator stretching should happen via scaling not stacking. So no render tree nodes would be created or destroyed. However, why is it bad to call setNeedsLayout on a descendent, and then do the layout (thus clearing the setNeedsLayout), all protected by a LayoutStateDisabler? (I'm asking an honest question, not a rhetorical one.) > > It's all about timing. There are certain phases of layout during which we don't want to modify the render tree, and there are certain invariants we want to enforce on some member functions of RenderObject in order to mitigate infinite recursion and ultimately security bugs. One of such invariant we'd like to have is to make computePreferredLogicalWidths a readonly operation. Not to beat a dead horse, but I'm asking why it has to be totally const. Why isn't not creating and deleting nodes good enough? I'm saying that the current scheme doesn't cause infinite recursion or security bugs, as far as I know. Am I wrong? > If we really need a bottom-up layout, we need to a major architectural change. Again, why? The current code does this, and there's no major architectural change. I keep asking, what is the problem it causes? > We need to address this problem ASAP. The problem is that, as you pointed out, there are very few people working on MathML but there are dozens if not hundreds of engineers working on the core rendering engine. And MathML is preventing this major refactoring effort to tighten constraints on computePreferredLogicalWidths, imposing a significant cost on the project. It's great that we have a working MathML implementation and there are people working hard to improve it, but we can't let the core code suffer either. What is the significant cost? How is the current code suffering? Seriously, I'll quit repeating my questions if you guys will quit repeating your answers. I get that you don't like bottom-up rendering, but not that it causes any actual problems. If it's too hard to explain the problems, or if they're secret for some reason, please say so. Created attachment 184612 [details]
Patch
Created attachment 184632 [details]
screenshot before patch
Created attachment 184633 [details]
screenshot after patch
(In reply to comment #38) > > If we really need a bottom-up layout, we need to a major architectural change. > > Again, why? The current code does this, and there's no major architectural change. I keep asking, what is the problem it causes? Historically, violating the "layout is top-down" assumption has led to crashes and security bugs. See http://trac.webkit.org/changeset/125262 as an example. The rendering code is sufficiently complicated that I can't come up with an exploit, but that doesn't mean there isn't one. Past experience indicates that there likely is at least a crash here if not an exploit. We'd like the code to be hardened against the possibility of such crashes because it's effectively impossible to prove correctness. This patch does not make doing the computeHeightWithoutLayout meaningfully harder and that appears to be the ideal end result. A patch doing that could modify the RenderMathMLRow code to remove the layoutIfNeeded and call computeHeightWithoutLayout on the children instead of logicalHeight. Then a followup patch can move that into computeIntrinsicLogicalWidths. That second patch would be partially undoing some of this patch, but very little of it. IMO, the patch makes our rendering look better in many cases and I don't know of any cases where it makes things look worse. Seems like the right short-term direction to me. Comment on attachment 184612 [details] Patch Attachment 184612 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://queues.webkit.org/results/16110639 New failing tests: mathml/differentiable-manifold.html (In reply to comment #42) > (In reply to comment #38) > > > If we really need a bottom-up layout, we need to a major architectural change. > > > > Again, why? The current code does this, and there's no major architectural change. I keep asking, what is the problem it causes? > > Historically, violating the "layout is top-down" assumption has led to crashes and security bugs. See http://trac.webkit.org/changeset/125262 as an example. The rendering code is sufficiently complicated that I can't come up with an exploit, but that doesn't mean there isn't one. Past experience indicates that there likely is at least a crash here if not an exploit. We'd like the code to be hardened against the possibility of such crashes because it's effectively impossible to prove correctness. Revision 125262 includes MarkOnlyThis in a few setNeedsLayout calls during layout. This makes sense. RenderMathMLBlock::computeChildrenPreferredLogicalHeights already does this in its setNeedsLayout call. Look, I (and David Carlisle) understand the need for security. At the end of last October, David Carlisle gave Abhishek ~3000 html pages with ~150000 mathml expressions to feed into the fuzzers. Abhishek said they were a great test suite. The fuzzers have been running for almost 3 months now, working from this, and we have one open security bug, which is independent of this patch. If a webkit or google security expert says that doing bottom-up layout is a serious security risk, based on his experience, I will immediately agree with this patch. But the truth is, you (Ojan) created this patch by mistakenly saying this was part of fixing the open security bug. When I pointed this out, you said MathML layout will never be good enough for us to care. Now you say it's a big security problem, but it's too hard to say why. Obviously this is a judgment call, but these are my reasons for my different judgment. I think the Ryosuke/Eric/Ojan position is really that folks working on the rendering code in general should not need to know or care about MathML specifics. I agree completely. However, you can't discard crucial MathML code that's been reviewed and tested just because it's different than HTML so it might produce a security problem, or we can never write new code. > This patch does not make doing the computeHeightWithoutLayout meaningfully harder and that appears to be the ideal end result. A patch doing that could modify the RenderMathMLRow code to remove the layoutIfNeeded and call computeHeightWithoutLayout on the children instead of logicalHeight. Then a followup patch can move that into computeIntrinsicLogicalWidths. That second patch would be partially undoing some of this patch, but very little of it. Eliminating the stuff from RenderMathMLBlock.h/cpp (and to a lesser extent RenderMathMLRoot.h/cpp) is a much bigger problem than RenderMathMLRow, and is not "very little" of this patch. The RenderMathMLBlock code took a long time to develop, is sophisticated and very helpful for MathML layout, and is close to the computeHeightWithoutLayout solution. Adding it back later to do computeHeightWithoutLayout will be a much bigger pain than keeping it active and updated with other code changes going forward. > IMO, the patch makes our rendering look better in many cases and I don't know of any cases where it makes things look worse. Seems like the right short-term direction to me. I think writing code changes a developer's perspective on requirements. If I told 100 knowledgeable people that you didn't know of any cases where making small operators as wide as large operators made MathML rendering worse, 99 would disagree (and really look at me funny, honestly). The most important next step for WebKit MathML is the operator dictionary, <http://www.w3.org/TR/MathML3/appendixc.html>, which basically gives default spacing around each of the mathematical operator characters in Unicode. This table would be pretty straightforward to add. Frédéric Wang has offered to help us port it from Gecko if we want. After this, MathML spacing would look pretty good, except for this patch, which would be very noticeably bad. What I really don't understand is why you are focusing on this, instead of fixing the open security bug. You have a patch to do that. Why don't we land it, without this part? We could discuss it more there, if necessary, but FWIW I agree with it if you don't remove this RenderMathMLBlock code also. Even better would be if you or Tony or someone would decide on the computeHeightWithoutLayout issue after that, and implement it here if necessary instead of just discarding the RenderMathMLBlock code here, but the security bug should be addressed first, IMHO. (In reply to comment #44) > > This patch does not make doing the computeHeightWithoutLayout meaningfully harder and that appears to be the ideal end result. A patch doing that could modify the RenderMathMLRow code to remove the layoutIfNeeded and call computeHeightWithoutLayout on the children instead of logicalHeight. Then a followup patch can move that into computeIntrinsicLogicalWidths. That second patch would be partially undoing some of this patch, but very little of it. > > Eliminating the stuff from RenderMathMLBlock.h/cpp (and to a lesser extent RenderMathMLRoot.h/cpp) is a much bigger problem than RenderMathMLRow, and is not "very little" of this patch. The RenderMathMLBlock code took a long time to develop, is sophisticated and very helpful for MathML layout, and is close to the computeHeightWithoutLayout solution. Adding it back later to do computeHeightWithoutLayout will be a much bigger pain than keeping it active and updated with other code changes going forward. I think you are overstating the amount of work it would be to add back in. It's all in the version control and we have tests. The hardest part is writing the computeHeightWithoutLayout function, which this patch does not make any harder. > What I really don't understand is why you are focusing on this, instead of fixing the open security bug. FWIW, I didn't start out trying to fix the security issue. I started out cleaning up computePreferredLogicalWidths to only be used for computing preferred widths and not for other side effects. I'm doing this throughout the render tree. This is a significant simplification of the code that makes the code easier to reason about and to work on. > You have a patch to do that. Why don't we land it, without this part? You and David Carlisle made it very clear that the rendering on that patch is not acceptable. In retrospect, I agree with you. But, now that I understand the problem better, it seems to me that what we really need is: 1. A computeHeightWithoutLayout function that we can use for computePreferredLogicalWidths. 2. A way of stretching operators in both the height and width directions without modifying the render tree. I'm not sure what this would look like, but it clearly would not look like my patch in bug 107197 since that only stretches them vertically and does so with a transform, which leads to unacceptably ugly results. Someone needs to take a look at how Mozilla is doing this to inform out implementation. If someone puts together either of these, I will happily review them. I really don't think bringing the RenderMathMLBlock code would be a lot of work once (1) and (2) above are completed. In fact, once those two patches are in, I volunteer write the patch myself to bring the RenderMathMLBlock code back. Then we will have good MathML rendering without violating any of the constraints of the render tree. (In reply to comment #45) > The hardest part is writing the computeHeightWithoutLayout function, which this patch does not make any harder. The RenderMathMLBlock code to compute and cache the preferred logical height (intrinsic height) is exactly what you're removing in this patch. That code could be modified to avoid using layout() if that is actually bad, to give computeHeightWithoutLayout. It's harder to write computeHeightWithoutLayout from scratch. > FWIW, I didn't start out trying to fix the security issue. I started out cleaning up computePreferredLogicalWidths to only be used for computing preferred widths and not for other side effects. I'm doing this throughout the render tree. This is a significant simplification of the code that makes the code easier to reason about and to work on. Patches to simplify code are good, but can't just throw away crucial parts of code that's already implemented, to make things simple. > > You have a patch to do that. Why don't we land it, without this part? > > You and David Carlisle made it very clear that the rendering on that patch is not acceptable. In retrospect, I agree with you. But, now that I understand the problem better, it seems to me that what we really need is: > 1. A computeHeightWithoutLayout function that we can use for computePreferredLogicalWidths. > 2. A way of stretching operators in both the height and width directions without modifying the render tree. I'm not sure what this would look like, but it clearly would not look like my patch in bug 107197 since that only stretches them vertically and does so with a transform, which leads to unacceptably ugly results. Someone needs to take a look at how Mozilla is doing this to inform out implementation. I respond to this in that bug, where we discuss security. But basically, I disagree with what you say here. > If someone puts together either of these, I will happily review them. > > I really don't think bringing the RenderMathMLBlock code would be a lot of work once (1) and (2) above are completed. In fact, once those two patches are in, I volunteer write the patch myself to bring the RenderMathMLBlock code back. Then we will have good MathML rendering without violating any of the constraints of the render tree. (1) above basically modifies the RenderMathMLBlock code to do its job with some new logic added. After doing (1) it'd be easy to add back the RenderMathMLBlock code, but would make no sense. Instead of cc'ing so many people, would it be better to discuss this by e-mail, or in the other bug? Committed r140880: <http://trac.webkit.org/changeset/140880> Re-opened since this is blocked by bug 108023 http://test-results.appspot.com/dashboards/flakiness_dashboard.html#group=%40ToT%20-%20chromium.org&tests=mathml%2Fdifferentiable-manifold.html%2Cmathml%2Fpresentation%2Ffenced-mi.xhtml%2Cmathml%2Fpresentation%2Ffenced.xhtml%2Cmathml%2Fpresentation%2Fmo-stretch.html%2Cmathml%2Fpresentation%2Fmo.xhtml%2Cmathml%2Fpresentation%2Fover.xhtml%2Cmathml%2Fpresentation%2Frow-alignment.xhtml%2Cmathml%2Fpresentation%2Frow.xhtml%2Cmathml%2Fpresentation%2Fsub.xhtml%2Cmathml%2Fpresentation%2Fsubsup.xhtml mathml/differentiable-manifold.html crash log for DumpRenderTree (pid 3558): STDOUT: <empty> STDERR: objc[3558]: Class MockCrApp is implemented in both /Volumes/data/b/build/slave/WebKit_Mac10_6__dbg_/build/src/xcodebuild/Debug/libwebkit.dylib and /Volumes/data/b/build/slave/WebKit_Mac10_6__dbg_/build/src/xcodebuild/Debug/DumpRenderTree.app/Contents/MacOS/DumpRenderTree. One of the two will be used. Which one is undefined. STDERR: ASSERTION FAILED: needsLayout() STDERR: ../../third_party/WebKit/Source/WebCore/rendering/RenderFlexibleBox.cpp(293) : virtual void WebCore::RenderFlexibleBox::layoutBlock(bool, WebCore::LayoutUnit) STDERR: 1 0x6928bd5d WebCore::RenderFlexibleBox::layoutBlock(bool, WebCore::LayoutUnit) STDERR: 2 0x6915d6b3 WebCore::RenderBlock::layout() STDERR: 3 0x694bc3c2 WebCore::RenderMathMLRow::stretchOperatorsAndLayout(int) STDERR: 4 0x694bc45f WebCore::RenderMathMLRow::layout() STDERR: 5 0x6916b8cd WebCore::RenderBlock::layoutBlockChild(WebCore::RenderBox*, WebCore::RenderBlock::MarginInfo&, WebCore::LayoutUnit&, WebCore::LayoutUnit&) STDERR: 6 0x691612a8 WebCore::RenderBlock::layoutBlockChildren(bool, WebCore::LayoutUnit&) STDERR: 7 0x6915ea77 WebCore::RenderBlock::layoutBlock(bool, WebCore::LayoutUnit) STDERR: 8 0x6915d6b3 WebCore::RenderBlock::layout() STDERR: 9 0x6916b8cd WebCore::RenderBlock::layoutBlockChild(WebCore::RenderBox*, WebCore::RenderBlock::MarginInfo&, WebCore::LayoutUnit&, WebCore::LayoutUnit&) STDERR: 10 0x691612a8 WebCore::RenderBlock::layoutBlockChildren(bool, WebCore::LayoutUnit&) STDERR: 11 0x6915ea77 WebCore::RenderBlock::layoutBlock(bool, WebCore::LayoutUnit) STDERR: 12 0x6915d6b3 WebCore::RenderBlock::layout() STDERR: 13 0x6916b8cd WebCore::RenderBlock::layoutBlockChild(WebCore::RenderBox*, WebCore::RenderBlock::MarginInfo&, WebCore::LayoutUnit&, WebCore::LayoutUnit&) STDERR: 14 0x691612a8 WebCore::RenderBlock::layoutBlockChildren(bool, WebCore::LayoutUnit&) STDERR: 15 0x6915ea77 WebCore::RenderBlock::layoutBlock(bool, WebCore::LayoutUnit) STDERR: 16 0x6915d6b3 WebCore::RenderBlock::layout() STDERR: 17 0x69473811 WebCore::RenderView::layoutContent(WebCore::LayoutState const&) STDERR: 18 0x69474234 WebCore::RenderView::layout() STDERR: 19 0x6a2647fd WebCore::FrameView::layout(bool) STDERR: 20 0x67f4961c WebCore::Document::implicitClose() STDERR: 21 0x6a0bcf82 WebCore::FrameLoader::checkCallImplicitClose() STDERR: 22 0x6a0bcafe WebCore::FrameLoader::checkCompleted() STDERR: 23 0x6a0bb2b3 WebCore::FrameLoader::finishedParsing() STDERR: 24 0x67f56d12 WebCore::Document::finishedParsing() STDERR: 25 0x6839b0a6 WebCore::HTMLConstructionSite::finishedParsing() STDERR: 26 0x683f3f83 WebCore::HTMLTreeBuilder::finished() STDERR: 27 0x683a5d96 WebCore::HTMLDocumentParser::end() STDERR: 28 0x683a3fc9 WebCore::HTMLDocumentParser::attemptToRunDeferredScriptsAndEnd() STDERR: 29 0x683a3d0c WebCore::HTMLDocumentParser::prepareToStopParsing() STDERR: 30 0x683a5e15 WebCore::HTMLDocumentParser::attemptToEnd() STDERR: 31 0x683a5f05 WebCore::HTMLDocumentParser::finish() mathml/presentation/mo-stretch.html crash log for DumpRenderTree (pid 29931): STDOUT: <empty> STDERR: SHOULD NEVER BE REACHED STDERR: ../../third_party/WebKit/Source/WebCore/rendering/mathml/RenderMathMLBlock.cpp(87) : virtual const char* WebCore::RenderMathMLBlock::renderName() const STDERR: 1 0x7f1d1a04f3c8 STDERR: 2 0x7f1d1a0275ff STDERR: 3 0x7f1d1a029d42 STDERR: 4 0x7f1d1a02a497 STDERR: 5 0x7f1d1a02af78 STDERR: 6 0x7f1d1a02b18a STDERR: Received signal 11 SEGV_MAPERR 0000bbadbeef STDERR: [0x7f1d1db427c0] base::debug::StackTrace::StackTrace() STDERR: [0x7f1d1db4206a] base::debug::(anonymous namespace)::StackDumpSignalHandler() STDERR: [0x7f1d1364c8f0] <unknown> STDERR: [0x7f1d1a04f3d2] WebCore::RenderMathMLBlock::renderName() STDERR: [0x7f1d1a0275ff] WebCore::RenderTreeAsText::writeRenderObject() STDERR: [0x7f1d1a029d42] WebCore::write() STDERR: [0x7f1d1a02a497] WebCore::write() STDERR: [0x7f1d1a02af78] WebCore::writeLayers() STDERR: [0x7f1d1a02b18a] WebCore::writeLayers() STDERR: r8: 00007f1d0c79c7e0 r9: 00007f1d1b91a61e r10: 00007f1d1945b228 r11: 0000000000000000 STDERR: r12: 0000000000000001 r13: 00007ffffe198a20 r14: 00007f1d0a355d58 r15: 00007f1d0c6eeec0 STDERR: di: 0000000000000000 si: 00000000efcdab90 bp: 00007ffffe1983e0 bx: 00007f1d0816f2b8 STDERR: dx: 00007f1d13637e00 ax: 00000000bbadbeef cx: 00007f1d13391acd sp: 00007ffffe1983c0 STDERR: ip: 00007f1d1a04f3d2 efl: 0000000000010246 cgf: 0000000000000033 erf: 0000000000000006 STDERR: trp: 000000000000000e msk: 0000000000000000 cr2: 00000000bbadbeef *** This bug has been marked as a duplicate of bug 122361 *** |