Bug 40986 - Change to Vertical Alignment of MathML Constructs to Use the Baseline
Summary: Change to Vertical Alignment of MathML Constructs to Use the Baseline
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit Misc. (show other bugs)
Version: 528+ (Nightly build)
Hardware: Macintosh Intel OS X 10.6
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks: 3251
  Show dependency treegraph
 
Reported: 2010-06-22 08:31 PDT by Alex Milowski
Modified: 2010-06-25 06:56 PDT (History)
1 user (show)

See Also:


Attachments
Patch to change to baseline alignment. (856.93 KB, patch)
2010-06-22 08:31 PDT, Alex Milowski
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Alex Milowski 2010-06-22 08:31:12 PDT
Created attachment 59371 [details]
Patch to change to baseline alignment.

This patch changes the way all MathML elements are rendered so the align against a calculated baseline.  This fixes a number of layout bugs and cleans up the implementation.

The patch is large because all the tests had to have their results reset.  The actual code changes are relatively small.
Comment 1 Kenneth Rohde Christiansen 2010-06-22 14:58:11 PDT
Comment on attachment 59371 [details]
Patch to change to baseline alignment.

LGTM.

165             int heightDiff = (m_scripts->offsetHeight() - maxHeight) / 2;
 166             if (heightDiff < 0) 
 167                 heightDiff = 0;

maybe add a comment to when this can happen?
Comment 2 Kenneth Rohde Christiansen 2010-06-22 14:59:39 PDT
Comment on attachment 59371 [details]
Patch to change to baseline alignment.

LGTM.

165             int heightDiff = (m_scripts->offsetHeight() - maxHeight) / 2;
 166             if (heightDiff < 0) 
 167                 heightDiff = 0;

maybe add a comment to when this can happen?

WebCore/mathml/RenderMathMLUnderOver.cpp:255
 +              baseline += static_cast<int>((zoomFactor > 1.25 ? 2 : 3) * zoomFactor);
ieck :-) good that you added a FIXME at least.
Comment 3 Alex Milowski 2010-06-23 02:42:10 PDT
(In reply to comment #1)
> (From update of attachment 59371 [details])
> LGTM.
> 
> 165             int heightDiff = (m_scripts->offsetHeight() - maxHeight) / 2;
>  166             if (heightDiff < 0) 
>  167                 heightDiff = 0;
> 
> maybe add a comment to when this can happen?

Only if the base is bigger than the scripts.   That shouldn't happen unless you are in the process of row height calculation.  I should add a comment to that effect.
Comment 4 Alex Milowski 2010-06-23 02:43:10 PDT
(In reply to comment #2)
> (From update of attachment 59371 [details])
> LGTM.
> 
> 165             int heightDiff = (m_scripts->offsetHeight() - maxHeight) / 2;
>  166             if (heightDiff < 0) 
>  167                 heightDiff = 0;
> 
> maybe add a comment to when this can happen?
> 
> WebCore/mathml/RenderMathMLUnderOver.cpp:255
>  +              baseline += static_cast<int>((zoomFactor > 1.25 ? 2 : 3) * zoomFactor);
> ieck :-) good that you added a FIXME at least.

There is some padding or margin that is expanding and I can't quite figure it out.  Very icky indeed.
Comment 5 WebKit Commit Bot 2010-06-25 06:55:56 PDT
Comment on attachment 59371 [details]
Patch to change to baseline alignment.

Clearing flags on attachment: 59371

Committed r61861: <http://trac.webkit.org/changeset/61861>
Comment 6 WebKit Commit Bot 2010-06-25 06:56:01 PDT
All reviewed patches have been landed.  Closing bug.