Bug 91171 - Chrome/Skia: PDF print output does not have clickable links.
Summary: Chrome/Skia: PDF print output does not have clickable links.
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Platform (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Steve VanDeBogart
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-07-12 17:32 PDT by Steve VanDeBogart
Modified: 2012-07-23 11:49 PDT (History)
5 users (show)

See Also:


Attachments
Patch (1.86 KB, patch)
2012-07-13 11:08 PDT, Steve VanDeBogart
no flags Details | Formatted Diff | Diff
Patch (2.29 KB, patch)
2012-07-13 14:00 PDT, Steve VanDeBogart
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from gce-cr-linux-03 (351.56 KB, application/zip)
2012-07-13 16:23 PDT, WebKit Review Bot
no flags Details
Patch (1.92 KB, patch)
2012-07-17 16:14 PDT, Steve VanDeBogart
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Steve VanDeBogart 2012-07-12 17:32:26 PDT
GraphicsContext::setURLForRect is not implemented for Skia so print output does not have clickable links.
Comment 1 Steve VanDeBogart 2012-07-13 11:08:22 PDT
Created attachment 152305 [details]
Patch
Comment 2 WebKit Review Bot 2012-07-13 11:25:51 PDT
Comment on attachment 152305 [details]
Patch

Attachment 152305 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/13242174
Comment 3 Stephen White 2012-07-13 13:28:55 PDT
Comment on attachment 152305 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=152305&action=review

Have you also rolled forward Chromium DEPS, so that the build.webkit.org builders don't break?

> Source/WebCore/platform/graphics/skia/GraphicsContextSkia.cpp:1000
>  void GraphicsContext::setURLForRect(const KURL& link, const IntRect& destRect)
>  {
> +    SkAutoDataUnref url(SkData::NewWithCString(link.string().utf8().data()));

Should this be protected by an if (paintingDisabled()) return; as is done on other platforms?
Comment 4 Steve VanDeBogart 2012-07-13 14:00:58 PDT
Created attachment 152325 [details]
Patch
Comment 5 Steve VanDeBogart 2012-07-13 14:02:58 PDT
Comment on attachment 152305 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=152305&action=review

>> Source/WebCore/platform/graphics/skia/GraphicsContextSkia.cpp:1000
>> +    SkAutoDataUnref url(SkData::NewWithCString(link.string().utf8().data()));
> 
> Should this be protected by an if (paintingDisabled()) return; as is done on other platforms?

Thanks. Done.

Also rolled the Chromium DEPS.
Comment 6 Stephen White 2012-07-13 14:52:09 PDT
Comment on attachment 152325 [details]
Patch

Looks good.  r=me
Comment 7 WebKit Review Bot 2012-07-13 16:23:44 PDT
Comment on attachment 152325 [details]
Patch

Attachment 152325 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/13242260

New failing tests:
jquery/manipulation.html
Comment 8 WebKit Review Bot 2012-07-13 16:23:47 PDT
Created attachment 152365 [details]
Archive of layout-test-results from gce-cr-linux-03

The attached test failures were seen while running run-webkit-tests on the chromium-ews.
Bot: gce-cr-linux-03  Port: <class 'webkitpy.common.config.ports.ChromiumXVFBPort'>  Platform: Linux-2.6.39-gcg-201203291735-x86_64-with-Ubuntu-10.04-lucid
Comment 9 Steve VanDeBogart 2012-07-17 16:14:03 PDT
Created attachment 152863 [details]
Patch
Comment 10 Stephen White 2012-07-18 06:45:04 PDT
Comment on attachment 152863 [details]
Patch

OK.  r=me
Comment 11 WebKit Review Bot 2012-07-18 10:10:53 PDT
Comment on attachment 152863 [details]
Patch

Clearing flags on attachment: 152863

Committed r122984: <http://trac.webkit.org/changeset/122984>
Comment 12 WebKit Review Bot 2012-07-18 10:10:59 PDT
All reviewed patches have been landed.  Closing bug.
Comment 13 Tony Chang 2012-07-18 11:10:32 PDT
This is causing a link failure on Chromium Win:

13>     Creating library ../../../../..\build\Debug\lib\webkit.lib and object ../../../../..\build\Debug\lib\webkit.exp
13>webcore_platform.lib(GraphicsContextSkia.obj) : error LNK2019: unresolved external symbol "public: static class SkData * __cdecl SkData::NewWithCString(char const * const)" (?NewWithCString@SkData@@SAPAV1@QBD@Z) referenced in function "public: void __thiscall WebCore::GraphicsContext::setURLForRect(class WebCore::KURL const &,class WebCore::IntRect const &)" (?setURLForRect@GraphicsContext@WebCore@@QAEXABVKURL@2@ABVIntRect@2@@Z)
13>../../../../..\build\Debug\webkit.dll : fatal error LNK1120: 1 unresolved externals
13>
13>Build FAILED.
Comment 14 Tony Chang 2012-07-18 11:21:40 PDT
Reverted r122984 for reason:

Broken the shared build, need to export a SkData in skia

Committed r122991: <http://trac.webkit.org/changeset/122991>
Comment 15 Steve VanDeBogart 2012-07-23 10:46:19 PDT
The fix for the shared build problem was committed in Skia 4658.  Chromium Deps is at 147759 which lists skia 4683 (>4658) as a dep.  So this should be fixed now.  What's the easiest way to reland the most recent patch?
Comment 16 Stephen White 2012-07-23 10:48:34 PDT
Comment on attachment 152863 [details]
Patch

Re-setting R+ and CQ+ should do it.  (r=me)
Comment 17 WebKit Review Bot 2012-07-23 11:49:00 PDT
Comment on attachment 152863 [details]
Patch

Clearing flags on attachment: 152863

Committed r123356: <http://trac.webkit.org/changeset/123356>
Comment 18 WebKit Review Bot 2012-07-23 11:49:05 PDT
All reviewed patches have been landed.  Closing bug.