Bug 138534 – Implement action menu support for videos

• Component: WebKit2
• Status: RESOLVED FIXED

[Beth Dakin]

Implement action menu support for videos

rdar://problem/18742164

[Beth Dakin]

Created attachment 241226 [details]
Patch

[WebKit Commit Bot]

Attachment 241226 [details] did not pass style-queue:


ERROR: Source/WebCore/WebCore.exp.in:0:  Source/WebCore/WebCore.exp.in should be sorted, use Tools/Scripts/sort-export-file script  [list/order] [5]
Total errors found: 1 in 11 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Tim Horton]

Comment on attachment 241226 [details]
Patch

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

> Source/WebCore/rendering/HitTestResult.cpp:508
> +bool HitTestResult::isMediaThatCanBeDownloaded() const

isDownloadableMedia?

> Source/WebKit2/UIProcess/mac/WKActionMenuController.mm:401
> +    RefPtr<WebHitTestResult> hitTestResult = WebHitTestResult::create(_hitTestResult.hitTestResult);

Shouldn't we just use hitTestResultForStage?

[Beth Dakin]

(In reply to comment #3)
> Comment on attachment 241226 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=241226&action=review
> 
> > Source/WebCore/rendering/HitTestResult.cpp:508
> > +bool HitTestResult::isMediaThatCanBeDownloaded() const
> 
> isDownloadableMedia?
> 

I like that better! Will use.

> > Source/WebKit2/UIProcess/mac/WKActionMenuController.mm:401
> > +    RefPtr<WebHitTestResult> hitTestResult = WebHitTestResult::create(_hitTestResult.hitTestResult);
> 
> Shouldn't we just use hitTestResultForStage?

UGH, I hate that enum. We should use that, except we don't know the stage here unless we pass it in, and doing that would be ridiculous. Because really, you just want the hitTestResultForStage method NOT to take an enum, and to return whichever hitTestResult it can access, favoring the new one. We have a lot of spots in this file that still choose the wrong hit test result (like all of the URL methods which all favor the old one!), I think I will fix this everywhere in a follow-up patch.

[Beth Dakin]

Thanks! http://trac.webkit.org/changeset/175779