Closed Bug 606109 Opened 9 years ago Closed 9 years ago

build fails with --disable-ogg --disable-wave

Categories

(Core :: DOM: Core & HTML, defect, critical)

x86
Linux
defect
Not set
critical

Tracking

()

RESOLVED FIXED
mozilla2.0b8
Tracking Status
blocking1.9.2 --- -
status1.9.2 --- wontfix

People

(Reporter: shtartora, Assigned: ehsan)

References

()

Details

Attachments

(1 file, 3 obsolete files)

User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.2.9) Gecko/20101021 Gentoo Namoroka/3.6.9
Build Identifier: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.2.9) Gecko/20101021 Thunderbird/3.1.4

Thunderbird do not build on my Gentoo GNU/Linux from official ebuild

Reproducible: Always

Steps to Reproduce:
1. emerge thunderbird
2. build is crash
Can you give us a stack trace ?
Severity: normal → critical
Keywords: crash, stackwanted
It is just don't build. There are build.log: http://paste.pocoo.org/show/278317/
by passing the distro is completely unacceptable IMO. I am aware that a small group of users is having an issue with current builds. You should check downstream bugzilla and cc yourself there, Ludovic feel free to close I will open a bug if appropriate when the issue is found.
Attached patch Missing idef MOZ_MEDIA (obsolete) — Splinter Review
Please assign for review this will address the bug.
Attachment #485019 - Flags: review?(Olli.Pettay)
This is only gonna effect 1.9.2 branch, mozilla-central is already using proper ifdef's for media.
Comment on attachment 485019 [details] [diff] [review]
Missing idef MOZ_MEDIA

Ehsan should review this since he added that code.
It is not clear to me whether we should allow
progress and loop also in TB even if it doesn't really support those elements.
Attachment #485019 - Flags: review?(Olli.Pettay) → review?(ehsan)
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Keywords: stackwanted
Product: Thunderbird → Core
QA Contact: general → general
We have addressed it with patch in gentoo, this is really only gonna be seen by those who disable ogg/wave support. I will continue to track the progress of this report, gentoo is out of here in terms of it being the culprit.
Comment on attachment 485019 [details] [diff] [review]
Missing idef MOZ_MEDIA

r=me, sorry for breaking things in the first place.
Attachment #485019 - Flags: review?(ehsan) → review+
(In reply to comment #6)
> It is not clear to me whether we should allow
> progress and loop also in TB even if it doesn't really support those elements.

With those #ifdef'ed out, we'll filter those elements out in the sanitizing fragment sink.
Keywords: crash
Attachment #485019 - Flags: approval1.9.2.12?
Comment on attachment 485019 [details] [diff] [review]
Missing idef MOZ_MEDIA

This needs to land on trunk first.
Attachment #485019 - Flags: approval1.9.2.12? → approval2.0?
(In reply to comment #10)
> Comment on attachment 485019 [details] [diff] [review]
> Missing idef MOZ_MEDIA
> 
> This needs to land on trunk first.

The first part is only relevant part of the patch for trunk, refer to 

http://mxr.mozilla.org/mozilla-central/source/content/base/src/nsContentSink.cpp#1882

loop is already handled via MOZ_MEDIA ifdef
I'm confused. "progress" is nothing to do with MOZ_MEDIA. It's an HTML5 interactive element.
(In reply to comment #12)
> I'm confused. "progress" is nothing to do with MOZ_MEDIA. It's an HTML5
> interactive element.

If that is the case Robert, then http://mxr.mozilla.org/mozilla-central/source/content/base/src/nsGkAtomList.h#1683 needs to be double checked.
(In reply to comment #13)
> (In reply to comment #12)
> > I'm confused. "progress" is nothing to do with MOZ_MEDIA. It's an HTML5
> > interactive element.
> 
> If that is the case Robert, then
> http://mxr.mozilla.org/mozilla-central/source/content/base/src/nsGkAtomList.h#1683
> needs to be double checked.

Yeah, that was why I thought that this change is correct...
Attachment #485019 - Flags: approval2.0? → approval2.0+
Comment on attachment 485019 [details] [diff] [review]
Missing idef MOZ_MEDIA

On second thought, this is the wrong thing to do.  The content sink should support these elements even if they're disabled in the build.  We should just define their respective atoms unconditionally.
Attachment #485019 - Flags: review-
Attachment #485019 - Flags: review+
Attachment #485019 - Flags: approval2.0+
Attached patch Pathc (v2) (obsolete) — Splinter Review
Like this.
Assignee: nobody → ehsan
Attachment #485019 - Attachment is obsolete: true
Attachment #489678 - Flags: review?(Olli.Pettay)
Blocks: 597791
Comment on attachment 489678 [details] [diff] [review]
Pathc (v2)

Strange place for progress. Put it higher up in the list, between
GK_ATOM(profile, "profile")
and
GK_ATOM(progressmeter, "progressmeter")
Attachment #489678 - Flags: review?(Olli.Pettay) → review+
Attached patch For check-in (obsolete) — Splinter Review
Attachment #489678 - Attachment is obsolete: true
Attachment #489920 - Flags: approval2.0?
Attached patch For check-inSplinter Review
New patch, as half of this was landed in http://hg.mozilla.org/mozilla-central/rev/1d6823f0c00f.
Attachment #489920 - Attachment is obsolete: true
Attachment #492085 - Flags: approval2.0?
Attachment #489920 - Flags: approval2.0?
Attachment #492085 - Flags: approval2.0? → approval2.0+
Whiteboard: [needs landing]
http://hg.mozilla.org/mozilla-central/rev/605656b6f779
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Whiteboard: [needs landing]
Target Milestone: --- → mozilla2.0b8
blocking1.9.2: --- → ?
status1.9.2: --- → ?
nsContentSink.cpp:1868: error: 'progress' is not a member of 'nsGkAtoms'
nsContentSink.cpp:1960: error: 'loop' is not a member of 'nsGkAtoms'
Assignee: ehsan → nobody
Component: General → DOM
QA Contact: general → general
Summary: Crash when i pretend a build thunderbird 3.1.5 → build fails with --disable-ogg --disable-wave
A non-default build option isn't going to "block" a release. Gentoo has already worked around it, it's fixed on trunk (although comment 21 is concerning), and the Thunderbird folks aren't asking for it so we'll "wontfix" on the branch without stronger arguments in favor.
blocking1.9.2: ? → -
(In reply to comment #21)

Which revision are you on?

> nsContentSink.cpp:1868: error: 'progress' is not a member of 'nsGkAtoms'

<http://mxr.mozilla.org/mozilla-central/source/content/base/src/nsGkAtomList.h#802>

> nsContentSink.cpp:1960: error: 'loop' is not a member of 'nsGkAtoms'

<http://mxr.mozilla.org/mozilla-central/source/content/base/src/nsGkAtomList.h#515>
Assignee: nobody → ehsan
ehsan: i'm pretty sure i copied that because i don't like relying on pastebins for relevant data.
(In reply to comment #24)
> ehsan: i'm pretty sure i copied that because i don't like relying on pastebins
> for relevant data.

What do you mean?  If this is answering comment 23, I don't know how to parse it.
Ehsan: trying again.

Bugs should be able to stand alone. Relying on third party paste bins for critical elements of a bug report -- especially what's actually broken (the symptoms) isn't cool.

To enable people reading a bug report to understand what was fixed, someone needs to put what was broken into the bug, that was missing from comment 0..comment 20. It's hidden in a paste.pocoo.org link which may or may not last (third party sites are known to die).

The changes I made for comment 21 -- https://bugzilla.mozilla.org/show_activity.cgi?id=606109  are in an effort to provide a meaningful bug summary and a meaningful bug description so that someone searching for bugs about problems can understand what was actually broken and thus understand why a given changeset actually fixed a given bug.

If this is unclear, please contact me via another means. I'm unlikely to be reading bugzilla in the future.
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.