[webkit-reviews] review denied: [Bug 37538] Implement page format data programming interface : [Attachment 58977] Implement page format data programming interface.

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Thu Jun 17 23:32:11 PDT 2010


Shinichiro Hamaji <hamaji at chromium.org> has denied Yuzo Fujishima
<yuzo at google.com>'s request for review:
Bug 37538: Implement page format data programming interface
https://bugs.webkit.org/show_bug.cgi?id=37538

Attachment 58977: Implement page format data programming interface.
https://bugs.webkit.org/attachment.cgi?id=58977&action=review

------- Additional Comments from Shinichiro Hamaji <hamaji at chromium.org>
WebCore/dom/Document.cpp:1517
 +	int x = style->marginLeft().calcValue(maxValue) +
style->borderLeft().width() + style->paddingLeft().calcValue(maxValue);
I'm not 100% sure, but maybe we want to make margin=0px when display is none.

WebCore/css/CSSStyleSelector.cpp:5516
 +	case 2:
We usually don't put a newline after case's colon?

WebCore/css/CSSStyleSelector.cpp:5534
 +		    if (!pageSizeFromName(primitiveValue0, primitiveValue1,
width, height))
Is this OK even for "size: a4 a4" ?

LayoutTests/printing/page-format-data-expected.txt:22
 +  PASS layoutTestController.preferredPageSizeInPixels(0) is "(816,1056)"
Cannot we make the results more readable? For example, inch(30) or something
like this?

LayoutTests/printing/page-format-data.html:120
 +  
We might want to test more invalid cases such as size: 100px 100px 100px, size:
-100px, etc.


More information about the webkit-reviews mailing list