[webkit-reviews] review denied: [Bug 10216] PDF created by printing should have live hyperlinks : [Attachment 10874] Will add HTML links to PDF output documents.

bugzilla-request-daemon at macosforge.org bugzilla-request-daemon at macosforge.org
Tue Oct 3 15:46:40 PDT 2006


Dave Hyatt <hyatt at apple.com> has denied Dave Hyatt <hyatt at apple.com>'s request
for review:
Bug 10216: PDF created by printing should have live hyperlinks
http://bugs.webkit.org/show_bug.cgi?id=10216

Attachment 10874: Will add HTML links to PDF output documents.
http://bugs.webkit.org/attachment.cgi?id=10874&action=edit

------- Additional Comments from Dave Hyatt <hyatt at apple.com>
Conflicts in the ChangeLog.

Style is wrong here:

+	 if(!primitiveValue) 
+	     break;
+	 if(primitiveValue->getIdent() == CSS_VAL__WEBKIT_LINK) {
+	     style->setOutlineAnnotation(true);
+	 }

Need spaces after the "ifs" and need to drop the braces from the second if.

A general observation here is that you've added way too many new tiny functions
to RenderObject that are no longer called in many places, so they can just be
folded right into the code that is adding the links.

You should not need to touch RenderBlock.cpp at all.  I do not understand any
of the changes in this file.

Outlines are painted in the two places I described in my earlier comment.  The
idea is that you only have to hook into those places and you can leverage the
rects that are collected by the outline code.

The new CSS property needs computed style implemented.	The boolean in
RenderStyle that you added to CSS3NonInheritedData needs to be initialized and
also added to the copy constructor and assignment operator.



More information about the webkit-reviews mailing list