Last Comment Bug 21479 - Embellished operators
: Embellished operators
Status: RESOLVED FIXED
: helpwanted
Product: Core
Classification: Components
Component: MathML (show other bugs)
: Trunk
: x86 Other
: -- normal (vote)
: mozilla5
Assigned To: Frédéric Wang (:fredw)
:
: Anthony Jones (:kentuckyfriedtakahe, :k17e)
Mentors:
Depends on: semantics 657279
Blocks: mathml-form
  Show dependency treegraph
 
Reported: 1999-12-10 18:54 PST by rbs
Modified: 2011-05-17 04:33 PDT (History)
9 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Screenshot for a tentative rendering of the MathML fragment (4.84 KB, image/gif)
1999-12-10 19:08 PST, rbs
no flags Details
testcase (986 bytes, text/xml)
2001-02-20 17:52 PST, rbs
no flags Details
testcase (1.02 KB, application/xhtml+xml)
2010-04-09 11:23 PDT, Frédéric Wang (:fredw)
no flags Details
testcase 2 (814 bytes, application/xhtml+xml)
2010-04-09 11:23 PDT, Frédéric Wang (:fredw)
no flags Details
Screenshot testcase 2 (13.17 KB, image/png)
2010-04-09 11:24 PDT, Frédéric Wang (:fredw)
no flags Details
Experimental patch (6.02 KB, patch)
2010-04-18 05:31 PDT, Frédéric Wang (:fredw)
no flags Details | Diff | Splinter Review
testcase 3 (1.18 KB, application/xhtml+xml)
2010-04-18 05:31 PDT, Frédéric Wang (:fredw)
no flags Details
add rules for mstyle, mphantom, mpadded and mrow (31.92 KB, patch)
2010-04-26 11:00 PDT, Frédéric Wang (:fredw)
karlt: review-
karlt: feedback+
Details | Diff | Splinter Review
add rules for mstyle, mphantom, mpadded and mrow (V2) (44.48 KB, patch)
2011-03-22 02:34 PDT, Frédéric Wang (:fredw)
karlt: review+
Details | Diff | Splinter Review
Final Patch [pushed] (44.50 KB, patch)
2011-03-23 00:21 PDT, Frédéric Wang (:fredw)
no flags Details | Diff | Splinter Review

Description rbs 1999-12-10 18:54:23 PST
Illustration of the newly added support for embellished operators.
More feedback is needed about the semantics in order to
ensure that the code does what it is supposed to be doing.

I will be attaching a screenshot for the MathML fragment
hereafter. The screenshot shows different rendering of <mover>
and <munder>. Which one is the expected rendering, while considering
that a particular subset can again be wrapped within various
other tags. For example, what should then be the rendering of
<msup> <mover>...</mover> <mi>x</mi> </msup>
<msub> <mover>...</mover> <mi>x</mi> </msub>

<math xmlns="http://www.w3.org/1998/Math/MathML">
  <mover>
    <mi>vectorrrrrrrrrrrrr</mi>
    <msubsup>
      <mo>&RightArrow;</mo>
      <mi>a</mi>
      <mi>b</mi>
    </msubsup>
  </mover>

  A special family of &Delta;-vector denoted as
  <mover>
    <mi>vectorrrrrrrrrrrrr</mi>
    <msub>
      <mo>&RightArrow;</mo>
      <mo>&Delta;</mo>
    </msub>
  </mover>

  <munder>
    <mi>vectorrrrrrrrrrrrr</mi>
    <msubsup>
      <mo>&RightArrow;</mo>
      <mi>a</mi>
      <mi>b</mi>
    </msubsup>
  </munder>

  A special family of &Delta;-vector denoted as
  <munder>
    <mi>vectorrrrrrrrrrrrr</mi>
    <msub>
      <mo>&RightArrow;</mo>
      <mo>&Delta;</mo>
    </msub>
  </munder>

</math>
Comment 1 rbs 1999-12-10 19:08:59 PST
Created attachment 3404 [details]
Screenshot for a tentative rendering of the MathML fragment
Comment 2 Gervase Markham [:gerv] 2001-01-12 14:38:06 PST
This bug has not been touched for more than nine months. In most cases, that 
means it has "slipped through the net". Please could the owner take a moment to 
add a comment to the bug with current status, and/or close it.

Thank you :-)

Gerv
Comment 3 rbs 2001-02-20 17:52:46 PST
Created attachment 25759 [details]
testcase
Comment 4 rbs 2001-02-20 18:09:48 PST
The current behavior is that, given the markup:
<munder>
  <mi>vectorrrrrrrrrrrrr</mi>
  <msub>
    <mo>&RightArrow;</mo>
    <mo>&Delta;</mo>
  </msub>
</munder>

The <mo>&RightArrow;</mo> is stretched to cover the base
<mi>vectorrrrrrrrrrrrr</mi>, then <msup> is constructed using the stretched
arrow, and so, after placing the superscript, the resulting width of the <msup>
is wider than the initial <mi>vectorrrrrrrrrrrrr</mi>, and therefore when
the two boxes are combined vertically by <munder>, they are centered by default
on the widest width, and we get what we see.

If there was an align attribute to disable the centering, the arrow could have
been aligned on the left edge of <mi>vectorrrrrrrrrrrrr</mi>. Haven't got
further feedback.
Comment 5 steve.swanson 2001-04-10 15:26:18 PDT
I feel that the current behavior is correct.  Centering is consistent and works
in general.  Any other scheme suffers difficulties once we leave the realm of
trivial examples.

Interestingly, the MathML spec. doesn't seem to specify anything about rendering
and the example they give only illustrates an unusual case.
Comment 6 rbs 2002-01-01 19:47:13 PST
Other examples provided by Steve: http://www.hipnt.com/MathML/embells.xml

Steve, do you now understand what is happening? Your examples had some questions.
These boil down to the explanation I gave earlier in comment #4. In each case in
your series, the thing to always keep in mind is that the stretchy element (if
taken in isolation) is sufficient to cover its base. Then, when combined with
the embellishments (scripts, etc), you get another box, and it is the resulting
box that is used subsequently for the centering. That's why the stretchy element
looks like it was "big". If the box was positioned differently, the stretchy
element would neatly sit above (or below) its base and there wouldn't be any
visual defect. But there is no model that will allow doing such positioning in a
consistent manner with complex markups as you noticed yourself in your previous
comment.

In your series, the cases where not much seems to happen are just because there
is no surrounding <mrow> beneath <math>. The plain usage of <math> has some
limitations w.r.t. stretchy elements due to the fact that <math> offers
linebreaking to the detriment of other MathML specific functions such as
balanced stretching of dangling pieces during linebreaking. A trade-off that I
have deemed okay for now.
Comment 7 rbs 2002-01-01 19:58:40 PST
There are some invalid examples such as the one with the question
"Labeled arrow. Why did it shrink?" The markup for that doesn't entitle the
inner arrow to stretch from outside its containing <mrow>. And BTW, it didn't
shring -- it retained its default size.
Comment 8 steve.swanson 2002-01-03 14:04:00 PST
Replying to #6: yes, I now understand what's happening here (after building a 
large matrix of examples).  The last thing to be understood what that the 
stretchy operators (arrows, at least) also make sure they are bigger than any 
embellishments they might have.  This behavior (which seems right) was 
interfering with my interpretation of other factors.

I didn't observe any different behavior after adding an <mrow> inside the outer 
<math>, though I do agree with comment #7 that some of my markup was amateurish.
Comment 9 rbs 2002-01-03 14:22:38 PST
>The last thing to be understood what that the stretchy operators (arrows, at
>least) also make sure they are bigger than any embellishments they might have.

This was a side-effect of the complex nature of the problem. It has been
improved by the tune-up that I checked in. Now, embellishments are selectively
included or excluded during size calculations whenever appropriate. Hence
stretchy operators are not always as big as any embellishments they might have,
unless in a situation where they should cover their embellishments as well.
Comment 10 Frédéric Wang (:fredw) 2010-04-09 11:23:23 PDT
Created attachment 438125 [details]
testcase

Add a doctype to the original testcase
Comment 11 Frédéric Wang (:fredw) 2010-04-09 11:23:51 PDT
Created attachment 438126 [details]
testcase 2
Comment 12 Frédéric Wang (:fredw) 2010-04-09 11:24:15 PDT
Created attachment 438127 [details]
Screenshot testcase 2
Comment 13 Frédéric Wang (:fredw) 2010-04-09 11:30:33 PDT
In addition to the alignment issue, here is what we should treated as embellished operators according to http://www.w3.org/TR/MathML3/chapter3.html#id.3.2.5.7.3

- first element of a semantics
- mstyle, mphantom, or mpadded used as mrow.
- mrow whose arguments consist (in any order) of one embellished operator and zero or more space-like elements.
Comment 14 Frédéric Wang (:fredw) 2010-04-18 05:31:13 PDT
Created attachment 439765 [details] [diff] [review]
Experimental patch

Just a test to check that we can pass the width of embellishments.
Comment 15 Frédéric Wang (:fredw) 2010-04-18 05:31:42 PDT
Created attachment 439766 [details]
testcase 3
Comment 16 Frédéric Wang (:fredw) 2010-04-18 05:32:54 PDT
BTW, it seems that the operator is does not always stretch to the exact size.
Comment 17 Frédéric Wang (:fredw) 2010-04-26 11:00:19 PDT
Created attachment 441538 [details] [diff] [review]
add rules for mstyle, mphantom, mpadded and mrow
Comment 18 Frédéric Wang (:fredw) 2010-04-26 11:02:15 PDT
Here is a patch that allows to handle the case where mstyle, mphantom, mpadded or mrow should be treated as embellished op. It also fixes the case of maction and contains some reftests.

As for Roger's initial issue, the MathML WG has chosen to make alignment according to the whole embellished op rather than the core (which is not too bad, since it will avoid us to complicate the code for very particular cases):
http://monet.nag.co.uk/~dpc/draft-spec/chapter3.html#id.3.4.6.2

Hence the only remaining issue before closing this bug is the case of semantics, which won't be too difficult once bug 556767 is fixed.
Comment 19 Frédéric Wang (:fredw) 2011-01-19 02:35:58 PST
(In reply to comment #17)
> Created attachment 441538 [details] [diff] [review]
> add rules for mstyle, mphantom, mpadded and mrow

Karl, can you please have a look to this patch before it becomes too old?
Comment 20 Karl Tomlinson (back Dec 13 :karlt) 2011-01-23 15:58:55 PST
I'll see what I can do, Frédéric.  I'm sorry it's been so long, but I'll need a good chunk of time to understand how this automatic data / embellished operator stuff works.
Comment 21 Karl Tomlinson (back Dec 13 :karlt) 2011-03-08 20:18:32 PST
Comment on attachment 441538 [details] [diff] [review]
add rules for mstyle, mphantom, mpadded and mrow

>+!= embellished-op-1.xhtml embellished-op-1-ref.xhtml
>+!= embellished-op-2.xhtml embellished-op-2-ref.xhtml
>+!= embellished-op-3.xhtml embellished-op-3-ref.xhtml

The files in the "!=" tests each contain several subtests.
For such "!=" tests to fail, all subtests would need to fail.
"!=" tests need each subtest to be in a separate file.

(I've started looking at the rest of the patch.)
Comment 22 Frédéric Wang (:fredw) 2011-03-19 13:52:30 PDT
Thank you Karl.

FYI, I'm developing a test framework for the MathJax project, which uses the principle of Mozilla's reftest. Hence I've started to write MathML reftests and plan to merge them with our own tests at some point. In particular, I had already split embellished-op into separate files (even the == one, although it is not needed) and this change will be integrated in the next version of my patch.
Comment 23 Karl Tomlinson (back Dec 13 :karlt) 2011-03-21 17:29:42 PDT
Comment on attachment 441538 [details] [diff] [review]
add rules for mstyle, mphantom, mpadded and mrow

// By convention, the data that we keep in this struct can change depending
// on any of our ancestors and/or descendants. If a data can be resolved
// solely from the embellished hierarchy, and it remains immutable once
// resolved, we put it in |nsEmbellishData|.

The SPACE_LIKE property seems similar to embellished properties, so, on
reading this, I wondered whether this bit should be stored on mEmbellishData
rather than mPresentationData.  But then even baseFrame is mPresentationData
(perhaps it could be on mEmbellishData) and SPACE_LIKE is not exactly
EmbellishData, so PresentationData seems fine.

>+  PRBool
>+  IsSpaceLike() {
>+    return NS_MATHML_IS_SPACE_LIKE(mPresentationData.flags);
>+  }

I think I'd prefer to have this implementation on nsMathMLFrame, as having
only the one function is simpler.

>+  NS_IMETHOD
>+  TransmitAutomaticDataForMrowLikeElement();

s/NS_IMETHOD/nsresult/
(NS_IMETHOD implies virtual and this doesn't need to be virtual.)

TransmitAutomaticDataForMrowLikeElement should unset NS_MATHML_SPACE_LIKE and
mPresentationData.baseFrame and mEmbellishData when the element becomes no
longer space-like or no-longer embellished because of descendant changes.
(InheritAutomaticData sets aPresentationData.flags = 0, but, looking at
nsMathMLContainerFrame::RebuildAutomaticDataForChildren, it looks like
InheritAutomaticData will not necessarily be called before
TransmitAutomaticData.)

Similarly for NS_MATHML_SPACE_LIKE in
nsMathMLmactionFrame::TransmitAutomaticData.

Would you mind adding a reftest to check that a vertically stretchy operator
stretches when embellished with an mspace?  IIUC I think that should render
identically whether or not the mspace is inside an mrow enclosing the mspace
and the stretchy operator.  (Prior to this patch we wouldn't stretch the
operator when in an mrow).
Comment 24 Frédéric Wang (:fredw) 2011-03-22 02:34:28 PDT
Created attachment 520883 [details] [diff] [review]
add rules for mstyle, mphantom, mpadded and mrow (V2)

Thanks again Karl.
I've update the patch and added reftests for vertical operators (the pages use an outer mrow because of bug 585347).
Comment 25 Karl Tomlinson (back Dec 13 :karlt) 2011-03-22 19:38:14 PDT
Comment on attachment 520883 [details] [diff] [review]
add rules for mstyle, mphantom, mpadded and mrow (V2)

Looks good, thanks!

embellished-op-3-4(-ref).html are identical to embellished-op-3-5(-ref).html
except for the comments.  Just need to change mphantom to mpadded in 3-4.

>+    // The element is not space-like
>+      mPresentationData.flags &= ~NS_MATHML_SPACE_LIKE;

Touch up the indentation on the second line here.
Comment 26 Frédéric Wang (:fredw) 2011-03-23 00:21:29 PDT
Created attachment 521124 [details] [diff] [review]
Final Patch [pushed]
Comment 27 Karl Tomlinson (back Dec 13 :karlt) 2011-03-27 19:49:05 PDT
Comment on attachment 521124 [details] [diff] [review]
Final Patch [pushed]

http://hg.mozilla.org/mozilla-central/rev/109cfae74006
Comment 28 Eric Shepherd [:sheppy] 2011-04-21 10:12:14 PDT
After reviewing this, it looks like it's just a rendering change rather than an API change, so I'm removing the doc-needed tag; please re-apply if there's an API or developer-impacting change I'm not seeing.

Note You need to log in before you can comment on or make changes to this bug.