Summary: | [Win] Crash in MathML layout test. | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Per Arne Vollan <pvollan> | ||||||
Component: | MathML | Assignee: | Per Arne Vollan <pvollan> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | CC: | bfulgham, commit-queue, mmaxfield, webkit-bug-importer | ||||||
Priority: | P2 | Keywords: | InRadar | ||||||
Version: | WebKit Nightly Build | ||||||||
Hardware: | Unspecified | ||||||||
OS: | Unspecified | ||||||||
Attachments: |
|
Description
Per Arne Vollan
2017-08-30 09:28:03 PDT
Created attachment 319374 [details]
Patch
Comment on attachment 319374 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=319374&action=review > Source/WebCore/rendering/mathml/MathOperator.cpp:641 > + return; It seems like these tests will still fail in Debug builds, right? If missing fonts is an expected condition, I think we should get rid of the ASSERTs. If the missing fonts are exceptional cases, we should probably add a LOG_ERROR or similar so we can catch cases of this in the field. Does this crash indicate that we are missing fonts or need some kind of setup on our test systems? Created attachment 319381 [details]
Patch
(In reply to Brent Fulgham from comment #2) > Comment on attachment 319374 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=319374&action=review > > > Source/WebCore/rendering/mathml/MathOperator.cpp:641 > > + return; > > It seems like these tests will still fail in Debug builds, right? > Yes. > If missing fonts is an expected condition, I think we should get rid of the > ASSERTs. If the missing fonts are exceptional cases, we should probably add > a LOG_ERROR or similar so we can catch cases of this in the field. > On macOS, this probably is an exceptional case. On Windows I think we should expect that this could happen, since the default MathML font support is more limited, as far as I know. > Does this crash indicate that we are missing fonts or need some kind of > setup on our test systems? Yes, we could most likely fix the crash on the bot by installing MathML fonts, but I also think it would be nice to have a fix for the crash. Thanks for reviewing! Comment on attachment 319381 [details]
Patch
r=me. I think we should try to make sure the test bots have whatever fonts they need to avoid hitting these ASSERTS, but it's probably good to keep them.
(In reply to Brent Fulgham from comment #5) > Comment on attachment 319381 [details] > Patch > > r=me. I think we should try to make sure the test bots have whatever fonts > they need to avoid hitting these ASSERTS, but it's probably good to keep > them. Thanks! I will look into installing MathML fonts on the bots. Comment on attachment 319381 [details] Patch Clearing flags on attachment: 319381 Committed r221394: <http://trac.webkit.org/changeset/221394> All reviewed patches have been landed. Closing bug. |