REGRESSION: printing is broken if stylesheet has @page

[Simon Fraser (smfr)]

Created attachment 58422 [details]
Testcase

http://whatsthepointy.blogspot.com/2010/06/safari-5-has-some-printing-problems.html

[Simon Fraser (smfr)]

<rdar://problem/8081713>

[Yuzo Fujishima]

Sorry for the regression. I'll look into this.

[Yuzo Fujishima]

Created attachment 58441 [details]
Fix Bug 40452: REGRESSION: printing is broken if stylesheet has @page

[Shinichiro Hamaji]

Comment on attachment 58441 [details]
Fix Bug 40452: REGRESSION: printing is broken if stylesheet has @page

WebCore/css/CSSStyleSelector.cpp:2795
 +                          if (childItem->isPageRule()) {
Cannot we create a function to do this so we can share code?

WebCore/ChangeLog:8
 +          No new tests because currently there is no easy way to test print layout.
I'm not 100% sure, but I guess we can test this patch by setPrinting though pixel dump isn't supported yet.

[Yuzo Fujishima]

Created attachment 58444 [details]
Refactored and added a test.

[Yuzo Fujishima]

Thank you for the review. Can you take another look?

(In reply to comment #4)
> (From update of attachment 58441 [details])
> WebCore/css/CSSStyleSelector.cpp:2795
>  +                          if (childItem->isPageRule()) {
> Cannot we create a function to do this so we can share code?

Agreed. Extracted a method.

> 
> WebCore/ChangeLog:8
>  +          No new tests because currently there is no easy way to test print layout.
> I'm not 100% sure, but I guess we can test this patch by setPrinting though pixel dump isn't supported yet.

Added a test.

[Shinichiro Hamaji]

Comment on attachment 58444 [details]
Refactored and added a test.

Looks good to me.

[Shinichiro Hamaji]

Ah, please add chromium's test_expectations.txt before you land.

[Yuzo Fujishima]

Committed r60992: <http://trac.webkit.org/changeset/60992>

[Shinichiro Hamaji]

Committed r63070: <http://trac.webkit.org/changeset/63070>