Bug 52733 - [Qt] Fix Layering violation in MediaPlayerPrivateQt...
Summary: [Qt] Fix Layering violation in MediaPlayerPrivateQt...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit Qt (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC All
: P5 Minor
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-01-19 12:29 PST by Dawit A.
Modified: 2011-01-20 10:24 PST (History)
1 user (show)

See Also:


Attachments
proposed patch... (1.10 KB, patch)
2011-01-19 12:31 PST, Dawit A.
kling: review-
Details | Formatted Diff | Diff
proposed patch II... (1.71 KB, patch)
2011-01-19 18:17 PST, Dawit A.
laszlo.gombos: review-
Details | Formatted Diff | Diff
proposed patch III... (1.72 KB, patch)
2011-01-19 23:21 PST, Dawit A.
eric: review+
Details | Formatted Diff | Diff
proposed patch IV... (1.91 KB, patch)
2011-01-20 07:48 PST, Dawit A.
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Dawit A. 2011-01-19 12:29:27 PST
The attached patch is inteded to fix a layering violation, usage of FrameLoaderClientQt, in WebCore/platform/graphics/MediaPlayerPrivateQt.cpp.
Comment 1 Dawit A. 2011-01-19 12:31:23 PST
Created attachment 79465 [details]
proposed patch...
Comment 2 Andreas Kling 2011-01-19 15:00:55 PST
Comment on attachment 79465 [details]
proposed patch...

Patch itself looks good, but we need a ChangeLog entry.
Comment 3 Dawit A. 2011-01-19 18:17:48 PST
Created attachment 79532 [details]
proposed patch II...

Added missing Changelog.
Comment 4 Laszlo Gombos 2011-01-19 19:47:32 PST
Comment on attachment 79532 [details]
proposed patch II...

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

r- as the ChangeLog needs to be fixed.

> Source/WebCore/ChangeLog:5
> +        [Qt] Fix Layering violation in MediaPlayerPrivateQt...

One dot please.

> Source/WebCore/ChangeLog:8
> +        No new tests. (OOPS!)

OOPS would need to go - from http://webkit.org/coding/contributing.html

The "No new tests. (OOPS!)" line appears if prepare-ChangeLog did not detect the addition of test cases. If your patch does not require test cases (or test cases are not possible), you should include a line stating such. Otherwise all changes require test cases which should be mentioned in the ChangeLog.
Comment 5 Dawit A. 2011-01-19 23:21:57 PST
Created attachment 79554 [details]
proposed patch III...

Fix changelog file some more...
Comment 6 Eric Seidel (no email) 2011-01-20 02:27:59 PST
Comment on attachment 79554 [details]
proposed patch III...

OK.  Can we remove the include of FrameLoaderClientQt.h?
Comment 7 Dawit A. 2011-01-20 07:48:36 PST
Created attachment 79601 [details]
proposed patch IV...

Removed the FrameLoaderClientQt.h include...
Comment 8 WebKit Commit Bot 2011-01-20 10:24:27 PST
Comment on attachment 79601 [details]
proposed patch IV...

Clearing flags on attachment: 79601

Committed r76265: <http://trac.webkit.org/changeset/76265>
Comment 9 WebKit Commit Bot 2011-01-20 10:24:31 PST
All reviewed patches have been landed.  Closing bug.