Remove obsolete OS X version checks in dom/media

RESOLVED FIXED in Firefox 50

Status

()

Core
Audio/Video: Playback
P3
normal
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: stefanh, Assigned: stefanh)

Tracking

Trunk
mozilla50
Unspecified
Mac OS X
Points:
---

Firefox Tracking Flags

(firefox50 fixed)

Details

Attachments

(1 attachment)

(Assignee)

Description

2 years ago
Created attachment 8767417 [details] [diff] [review]
Remove obsolete code

We don't support 10.6-10.8 anymore and I want to remove the OS X (now obsolete) version checks from nsCocoaFeatures.h (bug 1282251).
Attachment #8767417 - Flags: review?(cpearce)
To me there's a difference between no longer supporting a particular OS version and explicitly killing it. 

Removing this will kill video playback on 10.6
(Assignee)

Comment 2

2 years ago
I don't understand the reason to keep video playback on 10.6.

Firefox doesn't even start on < 10.9 anymore due to this: https://dxr.mozilla.org/mozilla-central/source/browser/app/macbuild/Contents/Info.plist.in#209 (same for all comm-central apps). And I doubt that any mozilla app will run on 10.6 even if you reverted that change, because a lot of code specific to 10.6-10.8 have already been removed in widget/cocoa.
(Assignee)

Comment 3

2 years ago
For code that's already been removed, see for example https://hg.mozilla.org/mozilla-central/rev/4ffe615e05a8 (sandboxing) and https://hg.mozilla.org/mozilla-central/rev/22f34da35017 (widget/cocoa).
Priority: -- → P2
Component: Audio/Video → Audio/Video: Playback
Comment on attachment 8767417 [details] [diff] [review]
Remove obsolete code

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

jesup can you look at the changes to dom/media/MediaManager.cpp please?

Anthony Jones assures me that we aren't updating existing Firefox installs on now-unsupported versions of MacOSX. So I'm fine with removing this code from Media Playback code.

This patch would have been significantly easier to review in mozreview. Please use that for review requests.

::: dom/media/platforms/apple/AppleVDADecoder.cpp
@@ +71,2 @@
>  #endif
> +  , mIs106(false)

Please remove mIs106 from AppleVDADecoder.h, and remove the "if (mIs106)" block in AppleVDADecoder::DoDecode(); we should never go down that path now.
Attachment #8767417 - Flags: review?(rjesup)
Attachment #8767417 - Flags: review?(cpearce)
Attachment #8767417 - Flags: review+
Mass change P2 -> P3
Priority: P2 → P3
Comment on attachment 8767417 [details] [diff] [review]
Remove obsolete code

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

::: dom/media/MediaManager.cpp
@@ +1876,5 @@
>        case MediaSourceEnum::Application:
>        case MediaSourceEnum::Window:
>          // Deny screensharing request if support is disabled, or
>          // the requesting document is not from a host on the whitelist, or
> +        // we're on WinXP until proved that it work

work -> works
Attachment #8767417 - Flags: review?(rjesup) → review+
(Assignee)

Comment 7

2 years ago
Thanks for the reviews. I haven't gotten to mozreview yet, but I guess I should...

Comment 8

2 years ago
Pushed by stefanh@inbox.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/a41833214621
Remove obsolete OS X version checks in dom/media. r=cpearce, jesup.

Comment 9

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/a41833214621
Status: ASSIGNED → RESOLVED
Last Resolved: 2 years ago
status-firefox50: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla50
You need to log in before you can comment on or make changes to this bug.