Bug 34195

Summary: CSS 'page-break-inside' property should be 'non-inherited' property.
Product: WebKit Reporter: Hayato Ito <hayato>
Component: CSSAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue, darin, hamaji, hayato, webkit.review.bot, yuzo
Priority: P3    
Version: 528+ (Nightly build)   
Hardware: PC   
OS: OS X 10.5   
URL: http://www.w3.org/TR/CSS2/page.html#page-break-props
Bug Depends on:    
Bug Blocks: 34080    
Attachments:
Description Flags
page-break-inside-non-inherited
none
page-break-inside-non-inherited-style-fixed
none
page-break-inside-add-test none

Description Hayato Ito 2010-01-26 20:10:01 PST
CSS 'page-break-inside' property is inherited from parent element in the current implementation.
It should be 'non-inherited' property according to CSS specification:
  
  http://www.w3.org/TR/CSS2/page.html#page-break-props

It seems that the CSS 'page-break-inside' property is not used at all when rendering.
So this is a not visible bug, but I'll use this property to implement the 'page-break-inside' feature in another bug here: https://bugs.webkit.org/show_bug.cgi?id=34080
Comment 1 Hayato Ito 2010-01-26 21:18:34 PST
Created attachment 47490 [details]
page-break-inside-non-inherited
Comment 2 WebKit Review Bot 2010-01-26 21:22:09 PST
Attachment 47490 [details] did not pass style-queue:

Failed to run "WebKitTools/Scripts/check-webkit-style" exit_code: 1
WebCore/rendering/style/RenderStyle.h:222:  Boolean expressions that span multiple lines should have their operators on the left side of the line instead of the right side.  [whitespace/operators] [4]
Total errors found: 1


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 3 Hayato Ito 2010-01-26 21:52:20 PST
Created attachment 47491 [details]
page-break-inside-non-inherited-style-fixed
Comment 4 Hayato Ito 2010-01-26 21:56:10 PST
I fixed the style issue for my change.

The file, RenderStyle.h,  has other style errors where I've not touched. I left them as is. If it is better to correct the other style errors, please let me know it.
Comment 5 Darin Adler 2010-01-27 12:58:11 PST
Comment on attachment 47491 [details]
page-break-inside-non-inherited-style-fixed

> +            return (_effectiveDisplay == other._effectiveDisplay)

If you're reformatting this, I suggest removing the unneeded parentheses as well.

This change needs a test case. review- because of the lack of a test case.
Comment 6 Darin Adler 2010-01-27 12:58:27 PST
Test case could involve getComputedStyle.
Comment 7 Hayato Ito 2010-01-28 01:29:21 PST
Created attachment 47599 [details]
page-break-inside-add-test
Comment 8 Hayato Ito 2010-01-28 02:01:02 PST
Thank you for the review!
I've removed the unneeded parentheses and added a test case.
Comment 9 Shinichiro Hamaji 2010-01-28 19:54:47 PST
Comment on attachment 47599 [details]
page-break-inside-add-test

cq+
Comment 10 WebKit Commit Bot 2010-01-28 23:01:49 PST
Comment on attachment 47599 [details]
page-break-inside-add-test

Clearing flags on attachment: 47599

Committed r54045: <http://trac.webkit.org/changeset/54045>
Comment 11 WebKit Commit Bot 2010-01-28 23:01:55 PST
All reviewed patches have been landed.  Closing bug.