Bug 152431 - Fix the !ENABLE(VIDEO) build after r192953 for <picture> element introduction
Summary: Fix the !ENABLE(VIDEO) build after r192953 for <picture> element introduction
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: HTML DOM (show other bugs)
Version: Other
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on: picture
Blocks:
  Show dependency treegraph
 
Reported: 2015-12-18 10:13 PST by Olivier Blin
Modified: 2015-12-18 11:38 PST (History)
9 users (show)

See Also:


Attachments
Patch (3.14 KB, patch)
2015-12-18 10:14 PST, Olivier Blin
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Olivier Blin 2015-12-18 10:13:12 PST
Since r192953, HTMLSourceElement is built even if video is disabled, since it is used by the picture element.
 
But this breaks build with -no-video, since HTMLMediaElement usage should be guarded by VIDEO guards, and its JS bindings were still under a video conditional.
Comment 1 Olivier Blin 2015-12-18 10:14:33 PST
Created attachment 267637 [details]
Patch
Comment 2 Alex Christensen 2015-12-18 10:50:55 PST
Comment on attachment 267637 [details]
Patch

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

> Source/WebCore/html/HTMLSourceElement.cpp:67
> +        else
> +#endif
> +        if (is<HTMLPictureElement>(*parent))

This is strange, but it's clear what is going on and has the correct behavior, and I can't make anything that does this that doesn't also look strange.
Comment 3 Olivier Blin 2015-12-18 11:21:05 PST
(In reply to comment #2)
> Comment on attachment 267637 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=267637&action=review
> 
> > Source/WebCore/html/HTMLSourceElement.cpp:67
> > +        else
> > +#endif
> > +        if (is<HTMLPictureElement>(*parent))
> 
> This is strange, but it's clear what is going on and has the correct
> behavior, and I can't make anything that does this that doesn't also look
> strange.

This is indeed a bit weird, but I didn't find a better way, and saw that this pattern is already used in a few places, like HTMLOptionElement.cpp

Thanks for the review!
Comment 4 WebKit Commit Bot 2015-12-18 11:38:21 PST
Comment on attachment 267637 [details]
Patch

Clearing flags on attachment: 267637

Committed r194278: <http://trac.webkit.org/changeset/194278>
Comment 5 WebKit Commit Bot 2015-12-18 11:38:27 PST
All reviewed patches have been landed.  Closing bug.