Bug 123543 - [MathML] The double bar vertical delimiter does not stretch properly
Summary: [MathML] The double bar vertical delimiter does not stretch properly
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: MathML (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Martin Robinson
URL:
Keywords: InRadar
Depends on: 122837
Blocks:
  Show dependency treegraph
 
Reported: 2013-10-30 16:58 PDT by Martin Robinson
Modified: 2013-11-23 09:05 PST (History)
10 users (show)

See Also:


Attachments
Patch (33.67 KB, patch)
2013-10-31 16:34 PDT, Martin Robinson
no flags Details | Formatted Diff | Diff
Patch for EWS (33.67 KB, patch)
2013-11-05 10:16 PST, Martin Robinson
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from webkit-ews-12 for mac-mountainlion-wk2 (479.64 KB, application/zip)
2013-11-05 11:07 PST, Build Bot
no flags Details
Archive of layout-test-results from webkit-ews-06 for mac-mountainlion (588.90 KB, application/zip)
2013-11-05 11:24 PST, Build Bot
no flags Details
Re-Upload of Patch with Mac Baseline (33.69 KB, patch)
2013-11-05 11:43 PST, Brent Fulgham
no flags Details | Formatted Diff | Diff
Rebased patch for Mac results (40.02 KB, patch)
2013-11-10 07:50 PST, Martin Robinson
bfulgham: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Martin Robinson 2013-10-30 16:58:38 PDT
It seems that the Unicode code point of this character is defined incorrectly in RenderMathMLOperator.cpp.
Comment 1 Martin Robinson 2013-10-31 16:11:16 PDT
The issue here seems to be that there are two vertical bars in Unicode ‖ (U+2016) and ∥ (U+2225). We don't handle the latter, which is the one that happens to be in the mathematical section, so we can trivially add support.
Comment 2 Martin Robinson 2013-10-31 16:34:20 PDT
Created attachment 215691 [details]
Patch
Comment 3 Radar WebKit Bug Importer 2013-11-05 09:38:09 PST
<rdar://problem/15394014>
Comment 4 Martin Robinson 2013-11-05 10:16:05 PST
Created attachment 216050 [details]
Patch for EWS
Comment 5 Brent Fulgham 2013-11-05 10:29:26 PST
Comment on attachment 216050 [details]
Patch for EWS

r=me
Comment 6 Brent Fulgham 2013-11-05 10:30:03 PST
If the EWS bots looks good, I think this should be landed.
Comment 7 Martin Robinson 2013-11-05 10:32:31 PST
I need to update Mac results since the test itself changed, unless you want to do it in another commit after the patch lands. Either way is fine with me. :)
Comment 8 Build Bot 2013-11-05 11:07:11 PST
Comment on attachment 216050 [details]
Patch for EWS

Attachment 216050 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.appspot.com/results/21158042

New failing tests:
mathml/presentation/mo-stretch.html
Comment 9 Build Bot 2013-11-05 11:07:13 PST
Created attachment 216052 [details]
Archive of layout-test-results from webkit-ews-12 for mac-mountainlion-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: webkit-ews-12  Port: mac-mountainlion-wk2  Platform: Mac OS X 10.8.5
Comment 10 Build Bot 2013-11-05 11:24:13 PST
Comment on attachment 216050 [details]
Patch for EWS

Attachment 216050 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.appspot.com/results/20608058

New failing tests:
mathml/presentation/mo-stretch.html
Comment 11 Build Bot 2013-11-05 11:24:16 PST
Created attachment 216054 [details]
Archive of layout-test-results from webkit-ews-06 for mac-mountainlion

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: webkit-ews-06  Port: mac-mountainlion  Platform: Mac OS X 10.8.5
Comment 12 Brent Fulgham 2013-11-05 11:43:32 PST
Created attachment 216056 [details]
Re-Upload of Patch with Mac Baseline
Comment 13 Martin Robinson 2013-11-10 07:50:17 PST
Created attachment 216525 [details]
Rebased patch for Mac results
Comment 14 Brent Fulgham 2013-11-11 12:20:29 PST
Comment on attachment 216525 [details]
Rebased patch for Mac results

r=me
Comment 15 Martin Robinson 2013-11-13 12:21:38 PST
Committed r159219: <http://trac.webkit.org/changeset/159219>
Comment 16 Frédéric Wang (:fredw) 2013-11-23 03:21:18 PST
I've been working on bug 99620 and stretch-mo is the only remaining failing test after my local changes. &DoubleVerticalBar; (U+2225) has a very confusing entity name, its Unicode name is actually "PARALLEL TO", that's an infix operator ("X is parallel to Y") not a delimiter. The MathML operator dictionary says it is not stretchy and not a fence.

Unfortunately, some people have used that character as a delimiter, instead of U+2016. So I'm wondering if we really want to make it stretchy... (IIRC, that's what Gecko does)...
Comment 17 Martin Robinson 2013-11-23 08:59:48 PST
(In reply to comment #16)
> I've been working on bug 99620 and stretch-mo is the only remaining failing test after my local changes. &DoubleVerticalBar; (U+2225) has a very confusing entity name, its Unicode name is actually "PARALLEL TO", that's an infix operator ("X is parallel to Y") not a delimiter. The MathML operator dictionary says it is not stretchy and not a fence.
> 
> Unfortunately, some people have used that character as a delimiter, instead of U+2016. So I'm wondering if we really want to make it stretchy... (IIRC, that's what Gecko does)...

Correct me if I'm wrong, but the operator dictionary just specifies the default value of the stretchy operator. The user can still override that by setting the stretchy attribute equal to true. I believe it's up to the user agent what operators stretch in that case. Perhaps the test should just have stretchy=true added to the parallel to operator.
Comment 18 Frédéric Wang (:fredw) 2013-11-23 09:05:35 PST
(In reply to comment #17)
> Correct me if I'm wrong, but the operator dictionary just specifies the default value of the stretchy operator. The user can still override that by setting the stretchy attribute equal to true. I believe it's up to the user agent what operators stretch in that case. Perhaps the test should just have stretchy=true added to the parallel to operator.

Yes, that's correct. I think we can add stretchy=true in the test if we really want to keep it stretchable. And I can add two stretchy prefix/postfix forms for that operator, for people who use that as open/close fences.