Follow up 828200 - the audio-channel-changed should follow comment 6 from bug 828200

RESOLVED FIXED in Firefox 21

Status

()

defect
RESOLVED FIXED
6 years ago
a month ago

People

(Reporter: baku, Assigned: baku)

Tracking

Trunk
mozilla21
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(blocking-b2g:tef+, firefox19 wontfix, firefox20 wontfix, firefox21 fixed, b2g18 fixed, b2g18-v1.0.0 fixed)

Details

Attachments

(1 attachment, 2 obsolete attachments)

Comment hidden (empty)
(Assignee)

Updated

6 years ago
Blocks: 828200
(Assignee)

Updated

6 years ago
Assignee: nobody → amarchesini
(Assignee)

Comment 1

6 years ago
Posted patch patch (obsolete) — Splinter Review
Attachment #702193 - Flags: review?(mchen)
Comment on attachment 702193 [details] [diff] [review]
patch

Review of attachment 702193 [details] [diff] [review]:
-----------------------------------------------------------------

The logic for finding higher audio channel type is good.
And please correct the right comments for unit test.

Thanks.

::: dom/audiochannel/tests/TestAudioChannelService.cpp
@@ +282,5 @@
> +  // Settings visible the normal channel.
> +  rv = normalAgent.mAgent->SetVisibilityState(true);
> +  NS_ENSURE_SUCCESS(rv, rv);
> +
> +  // Normal should not be playing because not visible.

// Normal should be playing because it is visible.

And the following comments for each channels are wrong too.
(Assignee)

Comment 3

6 years ago
Posted patch patch (obsolete) — Splinter Review
Attachment #702193 - Attachment is obsolete: true
Attachment #702193 - Flags: review?(mchen)
Attachment #702212 - Flags: review?(mchen)
Comment on attachment 702212 [details] [diff] [review]
patch

Review of attachment 702212 [details] [diff] [review]:
-----------------------------------------------------------------

Review+ after fixing the wrong comment.

Thanks.

::: dom/audiochannel/tests/TestAudioChannelService.cpp
@@ +336,5 @@
> +  // Settings visible the pNotification channel.
> +  rv = pNotificationAgent.mAgent->SetVisibilityState(true);
> +  NS_ENSURE_SUCCESS(rv, rv);
> +
> +  // pNotification should not be playing because not visible.

// pNotification should be playing because visible.
Attachment #702212 - Flags: review?(mchen) → review+
(Assignee)

Comment 5

6 years ago
Posted patch patchSplinter Review
Attachment #702212 - Attachment is obsolete: true
Attachment #702781 - Flags: review+
https://hg.mozilla.org/mozilla-central/rev/48461c81811e
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla21
(Assignee)

Updated

6 years ago
blocking-b2g: --- → tef?
(Assignee)

Updated

6 years ago
Blocks: 828283
(Assignee)

Comment 8

6 years ago
tef+ because bug 828283 needs this patch. AudioChannelService is used in b2g only so this patch should land on b2g18 too.
(In reply to Andrea Marchesini (:baku) from comment #8)
> tef+ because bug 828283 needs this patch. AudioChannelService is used in b2g
> only so this patch should land on b2g18 too.

Can you explain why this is a requirement for bug 828283?
(Assignee)

Comment 10

6 years ago
The patch for bug 828283 is based on the visibility of the media elements. In this patch we implement a visibility enum for each audio channel type.
Without this patch the visible-audio-channel-changed event will be emitted wrongly.
blocking-b2g: tef? → tef+
Summary: Follow up 828200 - the audio-channel-changed should follow comment 6 → Follow up 828200 - the audio-channel-changed should follow comment 6 from bug 828200
(Assignee)

Comment 13

6 years ago
https://hg.mozilla.org/releases/mozilla-b2g18/rev/c99fb2843bdc
Patch for 830648 was not landed yet. Pushed again to b2g18.
Landed on mozilla-b2g18/gaia master prior to the 1/25 branching to mozilla-b2g18_v1_0_0/v1.0.0, updating status-b2g-v1.0.0 to fixed.
Component: DOM → DOM: Core & HTML
Product: Core → Core
You need to log in before you can comment on or make changes to this bug.