Bug 117305 - MathML line fraction needs to parse number values
Summary: MathML line fraction needs to parse number values
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: MathML (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: chris fleizach
URL:
Keywords: WebExposed
Depends on: 118053
Blocks:
  Show dependency treegraph
 
Reported: 2013-06-06 09:07 PDT by chris fleizach
Modified: 2014-03-10 12:26 PDT (History)
10 users (show)

See Also:


Attachments
patch (11.91 KB, patch)
2013-06-06 18:03 PDT, chris fleizach
darin: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description chris fleizach 2013-06-06 09:07:58 PDT
If the line thickness of a <mfrac> object is missing, accessibility is not returning the correct default value.
Comment 1 chris fleizach 2013-06-06 09:28:00 PDT
This started off as an accessibility bug, but I noticed it would be better to pull the line thickness from the RenderMathMLFraction object. 

Then I saw that line thickness is not handling integer values like it should
Comment 2 chris fleizach 2013-06-06 18:03:48 PDT
Created attachment 203987 [details]
patch
Comment 3 Darin Adler 2013-06-07 07:12:32 PDT
Comment on attachment 203987 [details]
patch

OK
Comment 4 chris fleizach 2013-06-07 08:57:39 PDT
http://trac.webkit.org/changeset/151323
Comment 5 Frédéric Wang (:fredw) 2013-06-19 13:42:12 PDT
The correct syntax for MathML length is given here:
http://www.w3.org/TR/MathML/chapter2.html#type.length

Note that

"A number without a unit is intepreted as a multiple of the default value. This form is primarily for backward compatibility and should be avoided, prefering explicit units for clarity."

so strictly speaking your code should really do

m_lineThickness = thicknessIntValue * gLineMedium;

(it turns out that gLineMedium = 1.f so that does not change anything).

Anyway, I would have preferred that WebKit does not implement the deprecated unitless syntax for length or at least not before the recommended syntax with explicit unit is implemented.

Parsing of MathML lengths is implemented in bug 115610. I'm wondering why this patch took two days to be approved while my review request on bug 115610 has been ignored so far. Does the WebKit project require to ask explicit review on IRC or something in addition to the review flag?
Comment 6 chris fleizach 2013-06-19 13:48:16 PDT
(In reply to comment #5)
> The correct syntax for MathML length is given here:
> http://www.w3.org/TR/MathML/chapter2.html#type.length
> 
> Note that
> 
> "A number without a unit is intepreted as a multiple of the default value. This form is primarily for backward compatibility and should be avoided, prefering explicit units for clarity."
> 

http://www.w3.org/TR/REC-MathML/chap3_3.html#sec3.3.2

The examples all use unit less values and there is scant talk of deprecating that mode in that section. Anyone looking to set this is going to look at those examples first

> so strictly speaking your code should really do
> 
> m_lineThickness = thicknessIntValue * gLineMedium;
> 

Yes it would appear that is correct. Feel free to submit a patch

> (it turns out that gLineMedium = 1.f so that does not change anything).
> 
> Anyway, I would have preferred that WebKit does not implement the deprecated unitless syntax for length or at least not before the recommended syntax with explicit unit is implemented.

From the same paragraph

In general, the value of linethickness can be a number, as a multiplier of the default thickness of the fraction bar (the default thickness is not specified by MathML), or a number with a unit of vertical length (see Section 2.3.3), or one of the keywords medium (same as 1), thin (thinner than 1, otherwise up to the renderer), or thick (thicker than 1, otherwise up to the renderer).

-- There's no talk of deprecating unit less values.

> 
> Parsing of MathML lengths is implemented in bug 115610. I'm wondering why this patch took two days to be approved while my review request on bug 115610 has been ignored so far. Does the WebKit project require to ask explicit review on IRC or something in addition to the review flag?

Perhaps you have CC'd the wrong reviewers
Comment 7 Frédéric Wang (:fredw) 2013-06-19 14:09:08 PDT
> http://www.w3.org/TR/REC-MathML/chap3_3.html#sec3.3.2

> The examples all use unit less values and there is scant talk of deprecating > that mode in that section. Anyone looking to set this is going to look at > those examples first

> From the same paragraph

> In general, the value of linethickness can be a number, as a multiplier of the default thickness of the fraction bar (the default thickness is not specified by MathML), or a number with a unit of vertical length (see Section 2.3.3), or one of the keywords medium (same as 1), thin (thinner than 1, otherwise up to the renderer), or thick (thicker than 1, otherwise up to the renderer).

> -- There's no talk of deprecating unit less values.

This is the MathML 1 REC from 1998, the MathML 3 version does not have this paragraph (although it's true they kept the examples from the MathML 1 spec). The sentence I quoted is from the MathML 3 version: "This form is primarily for backward compatibility and should be avoided, prefering explicit units for clarity."

> Perhaps you have CC'd the wrong reviewers

It seems that Google people used to review the MathML patches last year. I asked elsewhere clarification about who is in charge of reviewing the MathML code after the Blink fork, but didn't get any reply. Thanks for having looked at my patch.
Comment 8 chris fleizach 2013-06-19 14:14:56 PDT
(In reply to comment #7)
> > http://www.w3.org/TR/REC-MathML/chap3_3.html#sec3.3.2
> 
> > The examples all use unit less values and there is scant talk of deprecating > that mode in that section. Anyone looking to set this is going to look at > those examples first
> 
> > From the same paragraph
> 
> > In general, the value of linethickness can be a number, as a multiplier of the default thickness of the fraction bar (the default thickness is not specified by MathML), or a number with a unit of vertical length (see Section 2.3.3), or one of the keywords medium (same as 1), thin (thinner than 1, otherwise up to the renderer), or thick (thicker than 1, otherwise up to the renderer).
> 
> > -- There's no talk of deprecating unit less values.
> 
> This is the MathML 1 REC from 1998, the MathML 3 version does not have this paragraph (although it's true they kept the examples from the MathML 1 spec). The sentence I quoted is from the MathML 3 version: "This form is primarily for backward compatibility and should be avoided, prefering explicit units for clarity."

Right you are. Well let's try to get this resolved with your patch

> 
> > Perhaps you have CC'd the wrong reviewers
> 
> It seems that Google people used to review the MathML patches last year. I asked elsewhere clarification about who is in charge of reviewing the MathML code after the Blink fork, but didn't get any reply. Thanks for having looked at my patch.

I'm not sure either but I'm willing to look at MathML patches
Comment 9 Frédéric Wang (:fredw) 2014-03-10 12:26:01 PDT
Mass change: add WebExposed keyword to help MDN documentation.