Display MathML fallback content when it is specified and take into account inline/display modes for…

Display MathML fallback content when it is specified and take into account inline/display modes for mathematical formulas. This is important for Web authors because Blink does not support MathML yet.

R=ojan@chromium.org,eseidel@chromium.org,jchaffraix@chromium.org
BUG=chromium:324764, chromium:152430
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=169439

[eseidel, 2014-01-15 19:29:15]

I'm OK with the idea.  I'm not sure if those are the right selectors to use.

Selectors in user agent stylesheets get applied to every page, so we should be
careful that they're as cheap as possible to compute.  In this case these won't
get applied to every page until someone browses a page with an element in the
matml namespace.  I don't know how many pages that will affect.

[esprehn, 2014-01-15 20:59:58]

not lgtm, you can't use + in a UA sheet and the other ones are universal rules.

https://codereview.chromium.org/138383003/diff/1/Source/core/css/mathml.css
File Source/core/css/mathml.css (right):

https://codereview.chromium.org/138383003/diff/1/Source/core/css/mathml.css#n...
Source/core/css/mathml.css:45: semantics > * {
This is bad, it ends up in the universal rules so we have to match it all the
time.

https://codereview.chromium.org/138383003/diff/1/Source/core/css/mathml.css#n...
Source/core/css/mathml.css:49: semantics > *:first-child + annotation-xml {
This is bad, it means we will invalidate a sibling of every SubtreeStyleChange
all the time because of the + operator.

https://codereview.chromium.org/138383003/diff/1/Source/core/css/mathml.css#n...
Source/core/css/mathml.css:54: math[alttext] * {
Another universal rule, it's going to add a rule to all elements in every page.

https://codereview.chromium.org/138383003/diff/1/Source/core/css/mathml.css#n...
Source/core/css/mathml.css:57: math[alttext]::after {
This is another universal rule :(

[fred.wang, 2014-02-01 10:07:37]

So as said by Eric, this stylesheet will only be loaded if someone goes on pages
that have MathML (which you said were negligible when you disabled the support
in Chrome). Besides these pages, there are many Websites using MathJax's
TeX/AsciiMath input but none serve MathML as an output to Chrome, so MathML
won't be inserted unless one uses the menu to switch to the MathML output mode.
The one I'm concerned is Wikipedia: in the future, it will always send
MathML+PNG+SVG output. However, for Chrome the MathML will always be in a
"display: none;" parent. Will the stylesheet still be inserted in that case (and
can that be avoided?). How will that affect the stylesheet performance?

Note that the universal selectors * used here have namespace
"http://www.w3.org/1998/Math/MathML", so my understanding is that even when the
stylesheet is loaded it will only impact the performance of Websites with MathML
elements, right? Or does Chrome add rules for non-MathML elements too? Also,
note that the alternative to that fallback method is to ask Web authors to use
an heavy library like MathJax, so the overhead added by the stylesheet is small,
in comparison.

I don't think I can implement the alttext without an universal rule so let's
ignore it. I think the semantics might be saved, but I suspect that would
require explicitly listing (almost) all the MathML elements so I'm not sure how
different it will be from the Universal selectors * currently used.

[fred.wang, 2014-02-06 17:37:14]

@esprehn Can you check this new version? This removes the universal selectors
and the + operator. However, many MathML elements are now explicitly specified
and the :nth-child pseudo class is used.

[esprehn, 2014-03-18 02:30:55]

lgtm, this seems fine.

[fred.wang, 2014-03-18 07:09:04]

The CQ bit was checked by fred.wang@free.fr

[commit-bot: I haz the power, 2014-03-18 07:09:10]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/fred.wang@free.fr/138383003/200001

[commit-bot: I haz the power, 2014-03-18 08:25:16]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-03-18 08:25:16]

Try jobs failed on following builders:
  tryserver.blink on linux_blink_dbg

[fred.wang, 2014-03-18 08:51:20]

On 2014/03/18 08:25:16, I haz the power (commit-bot) wrote:
> Try jobs failed on following builders:
>   tryserver.blink on linux_blink_dbg

So the Linux try failed here:

ASSERTION FAILED: m_defaultStyle->features().siblingRules.isEmpty()
../../third_party/WebKit/Source/core/css/CSSDefaultStyleSheets.cpp(182) : void
WebCore::CSSDefaultStyleSheets::ensureDefaultStyleSheetsForElement(WebCore::Element*,
bool&)

So does that mean I can't use :nth-child(2)?

I can remove :nth-child(2) and thus display *all* the annotations (but that
might not be what people want).

For some reason, it didn't assert on other debug builds...

[fred.wang, 2014-03-18 09:01:30]

The CQ bit was checked by fred.wang@free.fr

[commit-bot: I haz the power, 2014-03-18 09:01:37]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/fred.wang@free.fr/138383003/220001

[fred.wang, 2014-03-18 09:04:21]

The CQ bit was unchecked by fred.wang@free.fr

[fred.wang, 2014-03-18 09:08:24]

The CQ bit was checked by fred.wang@free.fr

[commit-bot: I haz the power, 2014-03-18 09:08:28]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/fred.wang@free.fr/138383003/240001

[esprehn, 2014-03-18 09:20:32]

On 2014/03/18 08:51:20, fred.wang wrote:
> On 2014/03/18 08:25:16, I haz the power (commit-bot) wrote:
> > Try jobs failed on following builders:
> >   tryserver.blink on linux_blink_dbg
> 
> So the Linux try failed here:
> 
> ASSERTION FAILED: m_defaultStyle->features().siblingRules.isEmpty()
> ../../third_party/WebKit/Source/core/css/CSSDefaultStyleSheets.cpp(182) : void
>
WebCore::CSSDefaultStyleSheets::ensureDefaultStyleSheetsForElement(WebCore::Element*,
> bool&)
> 
> So does that mean I can't use :nth-child(2)?
> 
> I can remove :nth-child(2) and thus display *all* the annotations (but that
> might not be what people want).
> 
> For some reason, it didn't assert on other debug builds...

Only the linux bot is actually running tests in debug, the rest of the debug
bots are just testing if it compiles. It does indeed seem you cannot use
nth-child in a UA sheet.

[fred.wang, 2014-03-18 09:28:17]

On 2014/03/18 09:20:32, esprehn wrote:
> Only the linux bot is actually running tests in debug, the rest of the debug
> bots are just testing if it compiles. It does indeed seem you cannot use
> nth-child in a UA sheet.

Thanks for the info. I submitted a new patch without the :nth-child that
displays all the annotations. This means that in practice, people will only be
able to use one annotation (unless they override the UA stylesheet of course).
That's fine however in most cases, I think. Typically, that would be just a TeX
source or image fallback.

[commit-bot: I haz the power, 2014-03-18 10:10:06]

Change committed as 169439