Last Comment Bug 760795 - build failure in nsGenericHTMLElement.cpp when MOZ_MEDIA is undefined
: build failure in nsGenericHTMLElement.cpp when MOZ_MEDIA is undefined
Product: Core
Classification: Components
Component: Audio/Video (show other bugs)
: Trunk
: x86 Linux
: -- normal (vote)
: mozilla16
Assigned To: Ralph Giles (:rillian) needinfo me
: Maire Reavy [:mreavy]
Depends on:
  Show dependency treegraph
Reported: 2012-06-02 05:43 PDT by :aceman
Modified: 2012-06-05 06:17 PDT (History)
4 users (show)
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---

patch (1.46 KB, patch)
2012-06-02 05:45 PDT, :aceman
bzbarsky: review-
Details | Diff | Splinter Review
alternate patch (6.46 KB, patch)
2012-06-02 07:50 PDT, Ralph Giles (:rillian) needinfo me
bzbarsky: review+
acelists: feedback+
Details | Diff | Splinter Review

Description :aceman 2012-06-02 05:43:17 PDT
/var/SSD/TB-hg/mozilla/content/html/content/src/nsGenericHTMLElement.cpp: In function 'bool IsVoidTag(mozilla::dom::Element*)':
/var/SSD/TB-hg/mozilla/content/html/content/src/nsGenericHTMLElement.cpp:1113:23: error: 'source' is not a member of 'nsGkAtoms'
gmake[8]: *** [nsGenericHTMLElement.o] Error 1
Comment 1 :aceman 2012-06-02 05:45:08 PDT
Created attachment 629464 [details] [diff] [review]
Comment 2 Ralph Giles (:rillian) needinfo me 2012-06-02 07:29:11 PDT
Would it be better to remove the #ifdef MOZ_MEDIA switches from the declaration in content/base/src/nsGkAtomList.h? The media-specific atoms are the only ones so conditionalized. And <track> is missing such a switch.

Is space in the atom list so precious that it's worth maintaining this configuration difference?
Comment 3 :aceman 2012-06-02 07:37:32 PDT
If you propose only remove the switches from the nsGkAtomList.h declaration but not the rest of the code (so that the atoms are defined but never used), that would solve the occasional build breakage when somebody forgets those atoms are conditional on MOZ_MEDIA.
Comment 4 Ralph Giles (:rillian) needinfo me 2012-06-02 07:50:36 PDT
Created attachment 629474 [details] [diff] [review]
alternate patch

Yes, I meant just removing the #ifdef MOZ_MEDIA from the list of defined atoms.
Comment 5 :aceman 2012-06-02 08:44:32 PDT
Comment on attachment 629474 [details] [diff] [review]
alternate patch

I can confirm this fixes the build too.

I do not use --disable-ogg (and others) to save run-time memory, but to not compile useless stuff (with build dependencies) and save compile time and ld memory. Because I build Thunderbird.
Comment 6 Ralph Giles (:rillian) needinfo me 2012-06-02 08:48:23 PDT
Comment on attachment 629474 [details] [diff] [review]
alternate patch

Ok, thanks. bz, what's your preference?
Comment 7 Boris Zbarsky [:bz] (still a bit busy) 2012-06-02 08:55:11 PDT
Comment on attachment 629474 [details] [diff] [review]
alternate patch

r=me, but note that the new DOM bindings for WebGLContext almost certainly don't compile without MOZ_MEDIA either: I was told that was not a supported configuration, and there is no real infrastructure in WebIDL to make methods conditional on compiler ifdefs at the moment...
Comment 8 Ralph Giles (:rillian) needinfo me 2012-06-02 08:59:12 PDT
Thanks. Ready to land.
Comment 9 :aceman 2012-06-02 09:09:42 PDT
For some reason it does work for me. Maybe TB does not compile webgl.
Comment 10 :aceman 2012-06-02 13:37:08 PDT
bz was right, after today's hg update WebGLRenderingContextBinding.cpp fails.
So, is there a --disable-webgl mozconfig option?
Comment 11 Boris Zbarsky [:bz] (still a bit busy) 2012-06-02 21:37:32 PDT
There isn't, sorry.

Note that once we convert 2d canvas to new bindings it would have the same problem: the canvas spec explicitly lists HTMLVideoElement as one of the overloads for drawImage.  And I really doubt there will be a --disable-2d-canvas option...
Comment 12 :aceman 2012-06-03 03:33:40 PDT
Fortunately removing --disable-ogg does work for me (it enables MOZ_MEDIA).
That is a pity the options are not there. I now must compile useless code (for TB). I'll try to file a bug for the options :)
Comment 13 Daniel Holbert [:dholbert] 2012-06-04 15:26:19 PDT
Comment 14 Ed Morley [:emorley] 2012-06-05 06:15:31 PDT
(This didn't make it in time for 15).
Comment 15 Geoff Lankow (:darktrojan) 2012-06-05 06:17:46 PDT

Note You need to log in before you can comment on or make changes to this bug.