Bug 159620

Summary: Move parsing of mpadded attributes to a MathMLPaddedElement class
Product: WebKit Reporter: Frédéric Wang (:fredw) <fred.wang>
Component: MathMLAssignee: Frédéric Wang (:fredw) <fred.wang>
Status: RESOLVED FIXED    
Severity: Normal CC: alex, bfulgham
Priority: P2    
Version: WebKit Nightly Build   
Hardware: All   
OS: All   
Bug Depends on: 155792, 156792    
Bug Blocks: 156536    
Attachments:
Description Flags
Patch
none
Patch bfulgham: review+

Frédéric Wang (:fredw)
Reported 2016-07-11 06:40:02 PDT
This is bug 156536 for the mpadded element.
Attachments
Patch (20.59 KB, patch)
2016-07-11 06:52 PDT, Frédéric Wang (:fredw) 		no flags
DetailsFormatted DiffDiff
Patch (20.19 KB, patch)
2016-07-12 09:46 PDT, Frédéric Wang (:fredw) 		bfulgham: review+
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Frédéric Wang (:fredw)
Comment 1 2016-07-11 06:52:14 PDT
Created attachment 283313 [details] Patch
Frédéric Wang (:fredw)
Comment 2 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.
Frédéric Wang (:fredw)
Comment 3 2016-07-12 09:46:16 PDT
Created attachment 283418 [details] Patch
Brent Fulgham
Comment 4 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?
Frédéric Wang (:fredw)
Comment 5 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.
Frédéric Wang (:fredw)
Comment 6 2016-07-12 21:47:53 PDT
Committed r203150: <http://trac.webkit.org/changeset/203150>
Note You need to log in before you can comment on or make changes to this bug.