Bug 136076 – Expose injected bundle SPI to get a node's URL element, get the visible selection range of that element, and snapshot that range

• Status: RESOLVED
• Resolution: FIXED
• Component: Layout and Rendering

[Peyton Randolph]

Expose injected bundle SPI to get a node's URL element, get the visible selection range of that element, and snapshot that range

[Peyton Randolph]

Created attachment 236814 [details]
Patch

Comment on attachment 236814 [details]
Patch

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

> Source/WebKit2/WebProcess/InjectedBundle/API/c/WKBundleNodeHandle.cpp:79
> +WKBundleNodeHandleRef WKBundleNodeHandleGetURLElementHandle(WKBundleNodeHandleRef elementHandle);
> +

What’s this? I think you left it in here by mistake.

> Source/WebKit2/WebProcess/InjectedBundle/API/c/WKBundleRangeHandle.cpp:36
> +using namespace WebCore;

We’ve moved away from using namespace WebCore in WebKit2. Just namespace-qualify what you use below.

> Source/WebKit2/WebProcess/InjectedBundle/DOM/InjectedBundleRangeHandle.cpp:136
> +    PaintBehavior cachedPaintBehavior = frameView->paintBehavior();

We normally call something like this “oldPaintBehavior” or “savedPaintBehavior”.

[Peyton Randolph]

Created attachment 236817 [details]
Patch

[Peyton Randolph]

(In reply to comment #2)
> (From update of attachment 236814 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=236814&action=review
> 
> > Source/WebKit2/WebProcess/InjectedBundle/API/c/WKBundleNodeHandle.cpp:79
> > +WKBundleNodeHandleRef WKBundleNodeHandleGetURLElementHandle(WKBundleNodeHandleRef elementHandle);
> > +
> 
> What’s this? I think you left it in here by mistake.

You're right. Removed.

> 
> > Source/WebKit2/WebProcess/InjectedBundle/API/c/WKBundleRangeHandle.cpp:36
> > +using namespace WebCore;
> 
> We’ve moved away from using namespace WebCore in WebKit2. Just namespace-qualify what you use below.

Removed.

> 
> > Source/WebKit2/WebProcess/InjectedBundle/DOM/InjectedBundleRangeHandle.cpp:136
> > +    PaintBehavior cachedPaintBehavior = frameView->paintBehavior();
> 
> We normally call something like this “oldPaintBehavior” or “savedPaintBehavior”.

Changed.

[Tim Horton]

Comment on attachment 236817 [details]
Patch

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

> Source/WebKit2/WebProcess/InjectedBundle/InjectedBundleHitTestResult.h:48
> +    PassRefPtr<InjectedBundleNodeHandle> URLElementHandle() const;

https://www.webkit.org/coding/coding-style.html#names-basic says that URL should be lowercase in all of these instances.

[Peyton Randolph]

Created attachment 236824 [details]
Patch

[Peyton Randolph]

(In reply to comment #5)
> (From update of attachment 236817 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=236817&action=review
> 
> > Source/WebKit2/WebProcess/InjectedBundle/InjectedBundleHitTestResult.h:48
> > +    PassRefPtr<InjectedBundleNodeHandle> URLElementHandle() const;
> 
> https://www.webkit.org/coding/coding-style.html#names-basic says that URL should be lowercase in all of these instances.

Fixed.

[Tim Horton]

Comment on attachment 236824 [details]
Patch

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

> Source/WebKit2/WebProcess/InjectedBundle/DOM/InjectedBundleRangeHandle.cpp:119
> +    float scaleFactor = (options & SnapshotOptionsExcludeDeviceScaleFactor) ? 1 : frame->page()->deviceScaleFactor();

It would be nice if we could share this middle block of code with InjectedBundleNodeHandle (and WebPage, etc.), but I'm not sure where to put it.

[WebKit Commit Bot]

Comment on attachment 236824 [details]
Patch

Rejecting attachment 236824 [details] from commit-queue.

Failed to run "['/Volumes/Data/EWS/WebKit/Tools/Scripts/webkit-patch', '--status-host=webkit-queues.appspot.com', '--bot-id=webkit-cq-01', 'build', '--no-clean', '--no-update', '--build-style=release', '--port=mac']" exit_code: 2 cwd: /Volumes/Data/EWS/WebKit

Last 500 characters of output:
0AnchorTypeE", referenced from:
      __ZNK6WebKit24InjectedBundleNodeHandle12visibleRangeEv in InjectedBundleNodeHandle.o
      __ZN7WebCore18lastPositionInNodeEPNS_4NodeE in InjectedBundleNodeHandle.o
ld: symbol(s) not found for architecture x86_64
clang: error: linker command failed with exit code 1 (use -v to see invocation)

** BUILD FAILED **


The following build commands failed:
	Ld /Volumes/Data/EWS/WebKit/WebKitBuild/Release/WebKit.framework/Versions/A/WebKit normal x86_64
(1 failure)

Full output: http://webkit-queues.appspot.com/results/4618171210268672

[Peyton Randolph]

Created attachment 236829 [details]
Patch

[WebKit Commit Bot]

Comment on attachment 236829 [details]
Patch

Clearing flags on attachment: 236829

Committed r172780: <http://trac.webkit.org/changeset/172780>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.