Closed
Bug 670334
Opened 13 years ago
Closed 10 years ago
Empty mfenced elements should display normal size of fences
Categories
(Core :: MathML, defect)
Core
MathML
Tracking
()
RESOLVED
FIXED
mozilla36
People
(Reporter: fs, Assigned: jkitch)
References
Details
Attachments
(3 files, 3 obsolete files)
(deleted),
text/html
|
Details | |
(deleted),
text/html
|
Details | |
(deleted),
patch
|
Details | Diff | Splinter Review |
Some follow-up information from bug 668969 and bug 553918:
> The height of the fences should be considered in containerSize and I don't
> see
> that happening. This might be a regression from bug 414277, because, before
> then, characters were always stretched to at least their base size.
> I think nsMathMLmfencedFrame::Reflow needs to get
> the normal size of the fences and consider them when setting containerSize.
See also the sample rendering in the MathML3 testsuite:
http://www.w3.org/Math/testsuite/build/main/Presentation/GeneralLayout/mfenced/mfenced1-full.xhtml
Also the attached test case could be used as a reftest.
Comment 1•13 years ago
|
||
@Florian: maybe the simplest way to solve this bug would be to directly return NS_OK when the base size is fine here:
http://mxr.mozilla.org/mozilla-central/source/layout/mathml/nsMathMLChar.cpp#1553
---
I also see issues with small roots badly displayed on Linux (probably due to the scale transform) and I think this change could fix the problem too:
http://www.maths-informatique-jeux.com/international/hsp/web/efficient_quantum_algorithms_and_HSP.php#id3.1.
Reporter | ||
Comment 2•13 years ago
|
||
(In reply to Frédéric Wang from comment #1)
> @Florian: maybe the simplest way to solve this bug would be to directly
> return NS_OK when the base size is fine here:
>
> http://mxr.mozilla.org/mozilla-central/source/layout/mathml/nsMathMLChar.
> cpp#1553
Thanks Frédéric, I've put that in a patch. Looks good to me too.
Comment 3•13 years ago
|
||
(In reply to Florian Scholz [:fs] from comment #2)
> Created attachment 612902 [details] [diff] [review]
> Patch V1
>
> (In reply to Frédéric Wang from comment #1)
> > @Florian: maybe the simplest way to solve this bug would be to directly
> > return NS_OK when the base size is fine here:
> >
> > http://mxr.mozilla.org/mozilla-central/source/layout/mathml/nsMathMLChar.
> > cpp#1553
>
> Thanks Frédéric, I've put that in a patch. Looks good to me too.
Thanks but my idea was just to replace "done = true;" by "return NS_OK;". If the size does not fit, we want to continue the stretching.
Reporter | ||
Comment 4•13 years ago
|
||
Attachment #612902 -
Attachment is obsolete: true
Attachment #613274 -
Flags: review?(karlt)
Attachment #612902 -
Flags: review?(karlt)
Comment 5•13 years ago
|
||
Comment on attachment 613274 [details] [diff] [review]
Patch V1.1
(In reply to Frédéric Wang from comment #1)
> I also see issues with small roots badly displayed on Linux (probably due to
> the scale transform)
I only see issues with small roots when building with --enable-system-cairo, which doesn't have the fix for bug 518172. I like the smaller roots. With some fonts I've seen the roots looking too large.
I also see this approach as not giving the optimal size in some other situations. e.g. when slightly larger than the base size is requested.
Is it possible to get the normal sizes of the fences by calling Stretch() on the nsMathMLChars with NS_STRETCH_NONE?
Attachment #613274 -
Flags: review?(karlt) → review-
Comment 6•13 years ago
|
||
> Is it possible to get the normal sizes of the fences by calling Stretch() on
> the nsMathMLChars with NS_STRETCH_NONE?
So you mean calling Stretch() twice, a first call with NS_STRETCH_NONE to get the normal size (which will be stored in charSize) before the current Stretch call:
http://mxr.mozilla.org/mozilla-central/source/layout/mathml/nsMathMLmfencedFrame.cpp#511
Comment 7•13 years ago
|
||
(In reply to Frédéric Wang from comment #6)
> > Is it possible to get the normal sizes of the fences by calling Stretch() on
> > the nsMathMLChars with NS_STRETCH_NONE?
>
> So you mean calling Stretch() twice,
Yes, but ...
> a first call with NS_STRETCH_NONE to
> get the normal size (which will be stored in charSize) before the current
> Stretch call:
>
> http://mxr.mozilla.org/mozilla-central/source/layout/mathml/
> nsMathMLmfencedFrame.cpp#511
... if mfenced were to behave like the spec'd nesting of mrows, and our mrow implementation, the children would be stretched as high as the separators. This would require getting the normal sizes in nsMathMLmfencedFrame::Reflow() rather than immediately before the stretch in ReflowChar().
For mfenced with one child, or where the children make their implied mrow an
embellished operator, the open and close characters would be considered in the
stretch applied to the child.
However, our mrow implementation doesn't quite follow the spec. According to
the spec, stretchy children should not be considered in the determining the
stretch size for vertical stretches unless there are no non-stretchy children.
(I personally don't see why stretchy and non-stretchy children should be
treated differently, nor why vertical stretching should be different from
horizontal.)
I don't think it is so important to follow the letter of the spec, but perhaps
we should think about what we really want to fix here.
Is the bug just that mfenced behaves differently from mrow, or do you think
that the current mfenced rendering is undesirable?
I remember now Frédéric's post including a comment about minsize.
http://lists.w3.org/Archives/Public/www-math/2012Jan/0003.html
Perhaps the best thing to do here is to set up a default minsize of 1?
That would resolve the issue for this testcase (even if it doesn't make
mfenced and mrow always behave similarly).
However, I don't think we have an effective minsize implementation.
Perhaps as a start, nsMathMLChar could be modified to never scale smaller than the base size.
I don't really mind if that ends up meaning that roots are always at least base size. If the smaller roots are preferred, we could add a stretch flag to behave differently for roots.
Reporter | ||
Updated•12 years ago
|
Assignee: elchi3 → nobody
Status: ASSIGNED → NEW
Comment 8•12 years ago
|
||
I was going to submit an mfenced bug but I suspect the issue is the same as this one, so adding a comment here, <mfenced><mo>·</mo></mfenced> shrinks the brackets to the size of the dot making it unreadable. I tried adding minsize but didn't seem to help. An online test case is
http://monet.nag.co.uk/~dpc/cdotbracket.html
(I'm on windows with stix fonts installed, using nightly 19.0a1 (2012-10-28)
In our production code I switch to <mrow><mo>(</mo> instead of mfenced, so a workaround is possible but it would be nice for the brackets not to shrink.
Comment 9•12 years ago
|
||
Neil, Bruce and other MathML WG members just think <mfenced> has a bad design and that <mrow>+<mo> should be preferred but they keep it in MathML because of some compromises with other community demands (it seems to be still necessary for Opera's implementation for instance). In Gecko, <mfenced> and <mrow>+<mo> are really implemented in disjoint code areas. This leads to inconsistencies and bugs like this one. I agree that since we implement mfenced, this bug should be fixed, but I'm not sure it is a top priority at the moment...
Comment 10•12 years ago
|
||
Comment 11•11 years ago
|
||
So this bug seems to be fixed for me(probably by the changes in bug 960115). I wonder if James' patch for bug 687807 also fixes it, even when the scale is applied.
I guess it's still worth extracting the reftest mfenced-11 from Florian's patch and integrate it into our test suite.
Assignee | ||
Comment 12•11 years ago
|
||
I can still reproduce this bug on Windows with the latest Nightly.
Assignee | ||
Updated•11 years ago
|
Assignee: nobody → jkitch.bug
Assignee | ||
Comment 13•10 years ago
|
||
This follows the suggestion from comment 6.
I have considered the suggestion from comment 7 by setting a minimum stretch size within nsMathMLChar::Stretch() on an opt-in basis, but this produced inferior metrics.
The issue is that the metrics for the stretchy character are manipulated before the Stretch() call.
https://hg.mozilla.org/mozilla-central/annotate/0ed32d9a42d6/layout/mathml/nsMathMLmfencedFrame.cpp#l299
If the minimum metrics are applied after the centering, the resulting metrics will probably not be centered appropriately and will look less like the <mrow> equivalent.
The attached testcase is somewhat wonky in that it doesn't work for all platforms.
Attachment #613274 -
Attachment is obsolete: true
Attachment #8503749 -
Flags: review?(karlt)
Comment 14•10 years ago
|
||
Comment on attachment 8503749 [details] [diff] [review]
patch
>+ApplyDefaultMetrics(nsPresContext* aPresContext,
What do you think about calling this "ApplyUnstretchedMetrics"?
Attachment #8503749 -
Flags: review?(karlt) → review+
Assignee | ||
Comment 16•10 years ago
|
||
Keywords: checkin-needed
Comment 17•10 years ago
|
||
Keywords: checkin-needed
Comment 18•10 years ago
|
||
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla36
Comment 19•10 years ago
|
||
Does this bug improves the situation with bug 727804?
Assignee | ||
Comment 20•10 years ago
|
||
Yes, separators are now included when considering fence size.
(Fences are also included when considering separator size, which doesn't quite follow the MathML standard, but bug 121748 decided that a stretch size of "everything" looked nicer).
Unfortunately the implementations of <mfenced> and <mrow>+<mo> are different enough that writing an automated test is problematic.
You need to log in
before you can comment on or make changes to this bug.
Description
•