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
Show Obsolete (3) View All Add an attachment (proposed patch, testcase, etc.)

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.