Bug 119546

Summary: [CSS Regions] clipping rectangle for "overflow: hidden" should be based on the padding box, not the content box
Product: WebKit Reporter: Mihai Maerean <mmaerean>
Component: WebCore Misc.Assignee: Radu Stavila <stavila>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue, esprehn+autocc, glenn, kondapallykalyan, stavila, stearns, WebkitBugTracker
Priority: P2 Keywords: AdobeTracked
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 57312    
Attachments:
Description Flags
repro file
none
patch
none
patch. incorporates the feedback: works with writing modes, defines font in tests, spelling.
none
Tests
none
Tests none

Mihai Maerean
Reported 2013-08-07 07:30:38 PDT
Created attachment 208266 [details] repro file CSS2.1, 11. Visual effects: Whenever overflow occurs, the 'overflow' property specifies whether a box is clipped to its padding edge http://www.w3.org/TR/CSS2/visufx.html
Attachments
repro file (428 bytes, text/html)
2013-08-07 07:30 PDT, Mihai Maerean 		no flags
Details
patch (12.12 KB, patch)
2013-08-27 05:48 PDT, Mihai Maerean 		no flags
DetailsFormatted DiffDiff
patch. incorporates the feedback: works with writing modes, defines font in tests, spelling. (28.54 KB, patch)
2013-08-29 08:22 PDT, Mihai Maerean 		no flags
DetailsFormatted DiffDiff
Tests (12.03 KB, patch)
2014-02-11 07:44 PST, Radu Stavila 		no flags
DetailsFormatted DiffDiff
Tests (12.03 KB, patch)
2014-02-11 08:09 PST, Radu Stavila 		no flags
DetailsFormatted DiffDiff
Show Obsolete (3) View All Add attachment proposed patch, testcase, etc.
Mihai Maerean
Comment 1 2013-08-27 05:48:41 PDT
Created attachment 209757 [details] patch
Radu Stavila
Comment 2 2013-08-27 06:11:07 PDT
Comment on attachment 209757 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=209757&action=review > Source/WebCore/ChangeLog:24 > + (WebCore::RenderRegion::transformedRectForClippingAtPaddingBox): Takes the clipping rectangle that's the size of Duplicate comment?
Andrei Bucur
Comment 3 2013-08-27 07:06:36 PDT
Comment on attachment 209757 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=209757&action=review Does it work with various writing modes? > LayoutTests/fast/regions/clip-to-padding-box-expected.html:8 > + font-size: 125%; I think explicitly setting the font size would be a good idea. > LayoutTests/fast/regions/clip-to-padding-box.html:9 > + margin-left: 3px; These values apply for other ports as well? > Source/WebCore/ChangeLog:10 > + Except for the 1st region, we can't allow clipping above the content rectangle because that would paint content You mean we can't allow painting? > Source/WebCore/ChangeLog:11 > + layed out in the previous region in the chain. layed -> laid > Source/WebCore/ChangeLog:27 > + layed out in the previous region in the chain. layed -> laid > Source/WebCore/rendering/RenderRegion.cpp:144 > + LayoutUnit deltaX = style()->overflowX() != OVISIBLE ? -paddingStart() : LayoutUnit(); I think you should use something like max(0, (paddingStart - (flowThreadPortionOverflowRect.x() - flowThreadPortionRect.x())) instead of paddingStart. You are painting from flowThreadPortionRect.x()/y() converted to regions coordinate while the regionClippingRect is expanded to account for the flowThreadPortionOverflowRect. Even better, uniting the regionClippingRect with the padding box of the region should give you the right result without too much math. The same for the the other deltas. > Source/WebCore/rendering/RenderRegion.cpp:147 > + // paint content layed out in the previous region in the chain. layed -> laid
Mihai Maerean
Comment 4 2013-08-29 08:22:56 PDT
Created attachment 209984 [details] patch. incorporates the feedback: works with writing modes, defines font in tests, spelling. to do: newly visible text should be selectable.
Mihai Maerean
Comment 5 2014-02-05 23:50:48 PST
Comment on attachment 209984 [details] patch. incorporates the feedback: works with writing modes, defines font in tests, spelling. View in context: https://bugs.webkit.org/attachment.cgi?id=209984&action=review > LayoutTests/ChangeLog:3 > + [CSS Regions] clipping rectangle for "overflow: hidden" should be based on the padding box, not the content box after the visual overflow patch, this patch doesn't pass the attached layout tests anymore.
Radu Stavila
Comment 6 2014-02-11 07:44:05 PST
Created attachment 223861 [details] Tests This problem was solved by https://bugs.webkit.org/show_bug.cgi?id=118665 and https://bugs.webkit.org/show_bug.cgi?id=128590. Two writing modes (horiz-bt and vert-rl) are still not working properly and a separate issue has been created for them (https://bugs.webkit.org/show_bug.cgi?id=128600).
Radu Stavila
Comment 7 2014-02-11 08:09:19 PST
Created attachment 223864 [details] Tests
WebKit Commit Bot
Comment 8 2014-02-11 09:28:40 PST
Comment on attachment 223864 [details] Tests Clearing flags on attachment: 223864 Committed r163880: <http://trac.webkit.org/changeset/163880>
WebKit Commit Bot
Comment 9 2014-02-11 09:28:42 PST
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.