Bug 159620 - Move parsing of mpadded attributes to a MathMLPaddedElement class
Summary: Move parsing of mpadded attributes to a MathMLPaddedElement class
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: MathML (show other bugs)
Version: WebKit Nightly Build
Hardware: All All
: P2 Normal
Assignee: Frédéric Wang (:fredw)
URL:
Keywords:
Depends on: 155792 156792
Blocks: 156536
  Show dependency treegraph
 
Reported: 2016-07-11 06:40 PDT by Frédéric Wang (:fredw)
Modified: 2016-07-12 21:47 PDT (History)
2 users (show)

See Also:


Attachments
Patch (20.59 KB, patch)
2016-07-11 06:52 PDT, Frédéric Wang (:fredw)
no flags Details | Formatted Diff | Diff
Patch (20.19 KB, patch)
2016-07-12 09:46 PDT, Frédéric Wang (:fredw)
bfulgham: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Frédéric Wang (:fredw) 2016-07-11 06:40:02 PDT
This is bug 156536 for the mpadded element.
Comment 1 Frédéric Wang (:fredw) 2016-07-11 06:52:14 PDT
Created attachment 283313 [details]
Patch
Comment 2 Frédéric Wang (:fredw) 2016-07-12 06:14:10 PDT
Note: This patch required bug 156792 to be fixed. Also, I wrote it on top of bug 159619 so it will still not apply to trunk of repo for now.
Comment 3 Frédéric Wang (:fredw) 2016-07-12 09:46:16 PDT
Created attachment 283418 [details]
Patch
Comment 4 Brent Fulgham 2016-07-12 09:58:31 PDT
Comment on attachment 283418 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=283418&action=review

Looks like a good refactoring. Feel free to land if there are no regressions.

> Source/WebCore/mathml/MathMLInlineContainerElement.cpp:-84
> -        return createRenderer<RenderMathMLPadded>(*this, WTFMove(style));

Good!

> Source/WebCore/rendering/mathml/RenderMathMLPadded.cpp:47
> +    // FIXME: Negative width values are not supported yet.

Is there a Bugzilla for this? If so, can you please note what it is here?

> Source/WebCore/rendering/mathml/RenderMathMLPadded.cpp:60
> +    if (height < 0)

Do we have a similar (to width) expectation to support negative height/depth/lspace in the future?
Comment 5 Frédéric Wang (:fredw) 2016-07-12 21:14:19 PDT
(In reply to comment #4)
> > Source/WebCore/rendering/mathml/RenderMathMLPadded.cpp:47
> > +    // FIXME: Negative width values are not supported yet.
> 
> Is there a Bugzilla for this? If so, can you please note what it is here?
> 
> > Source/WebCore/rendering/mathml/RenderMathMLPadded.cpp:60
> > +    if (height < 0)
> 
> Do we have a similar (to width) expectation to support negative
> height/depth/lspace in the future?

OK, just checked the spec and actually we miss negative lspace (not width/height/depth).

"Note that since a leading minus sign indicates a decrement, the size attributes (height, depth, width) cannot be set directly to negative values. In addition, specifying a decrement that would produce a net negative value for these attributes has the same effect as setting the attribute to zero. In other words, the effective bounding box of an mpadded element always has non-negative dimensions. However, negative values are allowed for the relative positioning attributes lspace and voffset."

I'll move this discussion to bug 85730.
Comment 6 Frédéric Wang (:fredw) 2016-07-12 21:47:53 PDT
Committed r203150: <http://trac.webkit.org/changeset/203150>