Bug 36897 - REGRESSION (r56429): Flash ads are clipped when main page is scrolled (boxofficemojo.com)
Summary: REGRESSION (r56429): Flash ads are clipped when main page is scrolled (boxoff...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Layout and Rendering (show other bugs)
Version: 528+ (Nightly build)
Hardware: Mac OS X 10.6
: P1 Normal
Assignee: Alexey Proskuryakov
URL:
Keywords: InRadar, Regression
Depends on:
Blocks:
 
Reported: 2010-03-31 12:09 PDT by Alexey Proskuryakov
Modified: 2010-03-31 13:12 PDT (History)
2 users (show)

See Also:


Attachments
proposed fix (6.25 KB, patch)
2010-03-31 12:40 PDT, Alexey Proskuryakov
mitz: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Alexey Proskuryakov 2010-03-31 12:09:19 PDT
1. Visit boxofficemojo.com
2. Scroll the page vertically

Results: The tops of the Flash ads at the top and along the right side of the page are clipped out.

This happens when using the version of Flash that currently ships with Mac OS X.

<rdar://problem/7804018>
Comment 1 Alexey Proskuryakov 2010-03-31 12:40:31 PDT
Created attachment 52197 [details]
proposed fix
Comment 2 mitz 2010-03-31 12:42:58 PDT
Comment on attachment 52197 [details]
proposed fix

Thanks for fixing this!

> +    IntRect m_clipRect; // The rectangle needs to remain correct after scrolling, so storing it in frame view coordinates, and not clipped to window.

I would say “content view coordinates”.

r=me
Comment 3 Eric Seidel (no email) 2010-03-31 12:44:10 PDT
Attachment 52197 [details] did not build on mac:
Build output: http://webkit-commit-queue.appspot.com/results/1612107
Comment 4 Darin Adler 2010-03-31 12:44:21 PDT
Comment on attachment 52197 [details]
proposed fix

> +#include "CString.h"

Please don't add this. Or you could state why you need to add it.

> -    IntRect m_windowClipRect;
> +    IntRect m_clipRect; // The rectangle needs to remain correct after scrolling, so storing it in frame view coordinates, and not clipped to window.

I would say "it is stored" rather than "storing it" here.
Comment 5 Alexey Proskuryakov 2010-03-31 12:49:18 PDT
Committed <http://trac.webkit.org/changeset/56858>.
Comment 6 Simon Fraser (smfr) 2010-03-31 13:06:25 PDT
Comment on attachment 52197 [details]
proposed fix


> Index: WebCore/rendering/RenderWidget.cpp

> +#include "CString.h"

Was this an intentional part of the change?
Comment 7 Alexey Proskuryakov 2010-03-31 13:12:45 PDT
Addressed additional review comments in <http://trac.webkit.org/changeset/56859>.