Bug 118601

Summary: Incorrect calculated width for mspace.
Product: WebKit Reporter: Frédéric Wang (:fredw) <fred.wang>
Component: MathMLAssignee: zalan <zalan>
Status: RESOLVED FIXED    
Severity: Normal CC: cfleizach, commit-queue, dbarton, esprehn+autocc, glenn, mrobinson, slewis, zalan
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Bug Depends on:    
Bug Blocks: 118812    
Attachments:
Description Flags
testcase
none
Patch
none
Patch
none
Patch for landing none

Description Frédéric Wang (:fredw) 2013-07-12 06:35:37 PDT
Created attachment 206536 [details]
testcase

See the testcase.

I guess

RenderMathMLSpace::computePreferredLogicalWidths()

should just set the specified width and should not call

RenderMathMLBlock::computePreferredLogicalWidths()
Comment 1 zalan 2013-07-16 02:18:29 PDT
Created attachment 206745 [details]
Patch
Comment 2 Frédéric Wang (:fredw) 2013-07-16 02:22:01 PDT
@Zalan I've cc'ed to two related bugs on MathML metrics. I think this one can be fixed independently but I just want to make you aware of Martin Robinson's refactoring of the MathML code that removes the "preferred heights" computation.
Comment 3 zalan 2013-07-16 02:23:07 PDT
(In reply to comment #2)
> @Zalan I've cc'ed to two related bugs on MathML metrics. I think this one can be fixed independently but I just want to make you aware of Martin Robinson's refactoring of the MathML code that removes the "preferred heights" computation.

good to know, thanks!
Comment 4 Frédéric Wang (:fredw) 2013-07-16 02:34:08 PDT
In the test: <div> and </span> do not match. I'm not sure why you need to use a margin style. What about using <span style="display: inline-block; width: ...; height: ...; background: ...;"> as in the other mspace reftests? Also do you really need a platform-specific reference file for a reftest (see bug 118599)?
Comment 5 zalan 2013-07-16 02:38:22 PDT
(In reply to comment #4)
> In the test: <div> and </span> do not match. I'm not sure why you need to use a margin style. What about using <span style="display: inline-block; width: ...; height: ...; background: ...;"> as in the other mspace reftests? Also do you really need a platform-specific reference file for a reftest (see bug 118599)?

I'll look into those. I also thought i didnt need the platform specific ref file for the reftest, but since the test framework was complaining, I added. I'll check again and will remove it. Thanks.
Comment 6 zalan 2013-07-16 06:15:40 PDT
Created attachment 206772 [details]
Patch
Comment 7 Stephanie Lewis 2013-07-17 17:29:55 PDT
Skipped Crashing tests in http://trac.webkit.org/changeset/152822 while waiting for a fix
Comment 8 chris fleizach 2013-07-17 18:11:06 PDT
Comment on attachment 206772 [details]
Patch

OK
Comment 9 Frédéric Wang (:fredw) 2013-07-17 22:14:30 PDT
Just a note: if bug 118812 is taken before that one, then you won't need to add the platform-specific reference for fractions. Of course that reference can also just be removed later.
Comment 10 zalan 2013-07-18 01:23:30 PDT
(In reply to comment #9)
> Just a note: if bug 118812 is taken before that one, then you won't need to add the platform-specific reference for fractions. Of course that reference can also just be removed later.

I'll just commit this and bug 118812 will take care of the cleanup of the platform specific files.
Comment 11 zalan 2013-07-18 01:26:29 PDT
Created attachment 206963 [details]
Patch for landing
Comment 12 Frédéric Wang (:fredw) 2013-07-18 01:27:09 PDT
(In reply to comment #10)

> I'll just commit this and bug 118812 will take care of the cleanup of the platform specific files.

ok, sounds good to me.
Comment 13 WebKit Commit Bot 2013-07-18 02:07:44 PDT
Comment on attachment 206963 [details]
Patch for landing

Clearing flags on attachment: 206963

Committed r152840: <http://trac.webkit.org/changeset/152840>
Comment 14 WebKit Commit Bot 2013-07-18 02:07:48 PDT
All reviewed patches have been landed.  Closing bug.