Bug 40452

Summary: REGRESSION: printing is broken if stylesheet has @page
Product: WebKit Reporter: Simon Fraser (smfr) <simon.fraser>
Component: PrintingAssignee: Yuzo Fujishima <yuzo>
Status: RESOLVED FIXED    
Severity: Normal CC: ap, hamaji, mitz
Priority: P1 Keywords: InRadar
Version: 528+ (Nightly build)   
Hardware: All   
OS: OS X 10.5   
Attachments:
Description Flags
Testcase
none
Fix Bug 40452: REGRESSION: printing is broken if stylesheet has @page
none
Refactored and added a test. hamaji: review+

Attachments
Testcase (4.18 KB, text/html)
2010-06-10 16:22 PDT, Simon Fraser (smfr) 		no flags
Details
Fix Bug 40452: REGRESSION: printing is broken if stylesheet has @page (2.21 KB, patch)
2010-06-10 20:20 PDT, Yuzo Fujishima 		no flags
DetailsFormatted DiffDiff
Refactored and added a test. (17.13 KB, patch)
2010-06-10 22:07 PDT, Yuzo Fujishima 		hamaji: review+
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Simon Fraser (smfr)
Comment 1 2010-06-10 16:23:02 PDT
<rdar://problem/8081713>
Yuzo Fujishima
Comment 2 2010-06-10 18:22:33 PDT
Sorry for the regression. I'll look into this.
Yuzo Fujishima
Comment 3 2010-06-10 20:20:29 PDT
Created attachment 58441 [details] Fix Bug 40452: REGRESSION: printing is broken if stylesheet has @page
Shinichiro Hamaji
Comment 4 2010-06-10 20:54:33 PDT
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
Comment 5 2010-06-10 22:07:54 PDT
Created attachment 58444 [details] Refactored and added a test.
Yuzo Fujishima
Comment 6 2010-06-10 22:09:54 PDT
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 7 2010-06-10 22:18:10 PDT
Comment on attachment 58444 [details] Refactored and added a test. Looks good to me.
Shinichiro Hamaji
Comment 8 2010-06-10 22:18:49 PDT
Ah, please add chromium's test_expectations.txt before you land.
Yuzo Fujishima
Comment 9 2010-06-10 22:51:55 PDT
Committed r60992: <http://trac.webkit.org/changeset/60992>
Shinichiro Hamaji
Comment 10 2010-07-12 04:19:00 PDT
Committed r63070: <http://trac.webkit.org/changeset/63070>
Note You need to log in before you can comment on or make changes to this bug.