Bug 65084 - Build without video but fullscreen API is broken
Summary: Build without video but fullscreen API is broken
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: DOM (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P3 Minor
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-07-24 13:58 PDT by Denis Oliver Kropp
Modified: 2011-07-28 07:12 PDT (History)
3 users (show)

See Also:

Attachments
Patch fixing missing isMediaElement() when building without video but fullscreen API (815 bytes, patch)
2011-07-24 13:58 PDT, Denis Oliver Kropp 		darin: review-
Details | Formatted Diff | Diff
Proper fix for the bug. (1.07 KB, patch)
2011-07-27 05:22 PDT, Denis Oliver Kropp 		no flags Details | Formatted Diff | Diff
Proper fix with ChangeLog entry. (1.78 KB, patch)
2011-07-27 05:38 PDT, Denis Oliver Kropp 		pnormand: review-
Details | Formatted Diff | Diff
Final bug fix (1.35 KB, patch)
2011-07-28 05:28 PDT, Denis Oliver Kropp 		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 Denis Oliver Kropp 2011-07-24 13:58:29 PDT 
Created attachment 101833 [details]
Patch fixing missing isMediaElement() when building without video but fullscreen API

Building without video support, but with fullscreen API is currently broken.

The virtual method isMediaElement() is not defined in that case.

Attached patch fixes this.
Comment 1 Darin Adler 2011-07-24 17:34:18 PDT 
Comment on attachment 101833 [details]
Patch fixing missing isMediaElement() when building without video but fullscreen API

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

> Source/WebCore/dom/Element.h:315
> -#if ENABLE(VIDEO)
> +#if ENABLE(VIDEO) || ENABLE(FULLSCREEN_API)

This is the wrong fix. The right fix is to put the code in EventDispatcher::determineDispatchBehavior inside this:

    #if ENABLE(FULLSCREEN_API) && ENABLE(VIDEO)
Comment 2 Evgeny Burmentyev 2011-07-27 05:14:40 PDT 
I build nightlies with --disable-video --disable-video-track, and I get the same error:

Source/WebCore/dom/EventDispatcher.cpp:387:22: error: ‘class WebCore::Element’ has no member named ‘isMediaElement’

...thus confirming the bug.
Comment 3 Denis Oliver Kropp 2011-07-27 05:22:40 PDT 
Created attachment 102132 [details]
Proper fix for the bug.
Comment 4 Philippe Normand 2011-07-27 05:28:16 PDT 
Comment on attachment 102132 [details]
Proper fix for the bug.

This looks good but lacks a ChangeLog entry. Please see http://www.webkit.org/coding/contributing.html :)
Comment 5 Denis Oliver Kropp 2011-07-27 05:38:06 PDT 
Created attachment 102133 [details]
Proper fix with ChangeLog entry.
Comment 6 Philippe Normand 2011-07-28 03:51:02 PDT 
Comment on attachment 102133 [details]
Proper fix with ChangeLog entry.

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

Almost there, thanks again for the patch!

> Source/WebCore/ChangeLog:3
> +        This fixes build without video enabled. Bug 65084.

You need to have 2 lines, one for the bug description and the second line containing the link to the bug.
prepare-ChangeLog --bug 65084 can be used too :)

> Source/WebCore/dom/EventDispatcher.cpp:9
> + * Copyright (C) 2011 Denis Oliver Kropp (dok@directfb.org)

According to our contributor policy only substantive code changes allow a copyright update. This patch is quite small, so please remove this line.
Comment 7 Denis Oliver Kropp 2011-07-28 05:28:09 PDT 
Created attachment 102245 [details]
Final bug fix
Comment 8 Philippe Normand 2011-07-28 05:59:54 PDT 
Comment on attachment 102245 [details]
Final bug fix

Thanks!
Comment 9 WebKit Review Bot 2011-07-28 07:12:53 PDT 
Comment on attachment 102245 [details]
Final bug fix

Clearing flags on attachment: 102245

Committed r91923: <http://trac.webkit.org/changeset/91923>
Comment 10 WebKit Review Bot 2011-07-28 07:12:57 PDT 
All reviewed patches have been landed.  Closing bug.