Bug 111932

Summary: Add a heuristic to determine the “primary” snapshotted plugin
Product: WebKit Reporter: Tim Horton <thorton>
Component: Plug-insAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: dino, esprehn+autocc, jonlee, ojan.autocc, sam, simon.fraser, webkit-ews, webkit.review.bot
Priority: P2 Keywords: InRadar
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on: 145252    
Bug Blocks:    
Attachments:
Description Flags
patch
none
patch
webkit-ews: commit-queue-
patch
none
patch dino: review+

Description Tim Horton 2013-03-09 17:34:45 PST
So it could be styled differently, etc. via a UA stylesheet.

<rdar://problem/13270208>
Comment 1 Tim Horton 2013-03-09 17:39:09 PST
Putting it in WK2 because that’ll keep it out of the way for most people... while a heuristic like this doesn’t really belong in WebKit at all - it’s going to require a good bit of effort to make it possible to put it in the client. We plan to eventually expose enough knowledge to do that, but not yet.
Comment 2 Tim Horton 2013-03-09 18:09:51 PST
Created attachment 192362 [details]
patch
Comment 3 Tim Horton 2013-03-09 18:12:35 PST
Created attachment 192363 [details]
patch
Comment 4 Early Warning System Bot 2013-03-09 18:18:34 PST
Comment on attachment 192363 [details]
patch

Attachment 192363 [details] did not pass qt-wk2-ews (qt):
Output: http://webkit-commit-queue.appspot.com/results/17009871
Comment 5 EFL EWS Bot 2013-03-09 18:19:12 PST
Comment on attachment 192363 [details]
patch

Attachment 192363 [details] did not pass efl-ews (efl):
Output: http://webkit-commit-queue.appspot.com/results/17122184
Comment 6 Tim Horton 2013-03-09 18:21:54 PST
Created attachment 192364 [details]
patch
Comment 7 Sam Weinig 2013-03-09 19:13:40 PST
Can we use a different #define that PLATFORM(MAC) for this, at the very least to be more descriptive.
Comment 8 Tim Horton 2013-03-09 19:14:52 PST
(In reply to comment #7)
> Can we use a different #define that PLATFORM(MAC) for this, at the very least to be more descriptive.

Yeah, I was considering that. Currently trying to consolidate all of the #ifs in one file so that that becomes less touch-Platform.h and more add-something-to-the-top-of-WebPage.
Comment 9 Tim Horton 2013-03-09 19:25:36 PST
(In reply to comment #8)
> (In reply to comment #7)
> > Can we use a different #define that PLATFORM(MAC) for this, at the very least to be more descriptive.
> 
> Yeah, I was considering that. Currently trying to consolidate all of the #ifs in one file so that that becomes less touch-Platform.h and more add-something-to-the-top-of-WebPage.

I’m having a hard time telling if that’s a legitimate thing to do. People seem to be using WTF_USE_ for that, which I think is wrong, it should be WTF_ENABLE_, and it’s not done very often. I’ll move it to Platform.h/wherever if someone complains.
Comment 10 Tim Horton 2013-03-09 19:37:07 PST
Created attachment 192367 [details]
patch
Comment 11 Dean Jackson 2013-03-10 13:36:32 PDT
Comment on attachment 192367 [details]
patch

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

> Source/WebKit2/WebProcess/WebPage/WebPage.cpp:3953
> +            LayoutUnit contentArea = renderBox->contentWidth() * renderBox->contentHeight();
> +
> +            if (renderBox->contentWidth() < primarySnapshottedPlugInMinimumWidth || renderBox->contentHeight() < primarySnapshottedPlugInMinimumHeight)
> +                continue;

You can swap these two statements.
Comment 12 Tim Horton 2013-03-10 13:52:56 PDT
http://trac.webkit.org/changeset/145332