Closed Bug 1215582 Opened 4 years ago Closed 4 years ago

Rename Blacklist to Blocklist in GstreamerFormatHelper

Categories

(Core :: Audio/Video: Playback, defect, P2)

defect

Tracking

()

RESOLVED FIXED
mozilla44
Tracking Status
firefox44 --- fixed

People

(Reporter: rillian, Assigned: rillian)

References

Details

Attachments

(1 file, 2 obsolete files)

The term 'Blacklist' always bothered me. Use the less colour-related 'Blocklist' term instead.
Attached patch Rename patch (obsolete) — Splinter Review
Attachment #8674980 - Flags: review?(gsquelart)
Comment on attachment 8674980 [details] [diff] [review]
Rename patch

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

r+, but you should also wchange the remaining "blacklist"'s in some comments:
GStreamerReader.cpp:1209: * blacklist.
GStreamerReader.cpp:1246: * candidate factories to continue decoding the stream. We apply the blacklist
GStreamerReader.h:96:   * constructed, we can then list for its autoplug-sort signal to blacklist
GStreamerReader.h:197:   * container/codec blacklist.

And to address Anthony's comment:
(In reply to Anthony Jones (:kentuckyfriedtakahe, :k17e) from comment #2)
> Blacklist has the virtue of being an actual word.
> http://www.oxforddictionaries.com/spellcheck/english/?q=blocklist
You could capitalize the 'L' or separate words, e.g.: sBlockListEnabled, block list.

::: dom/media/gstreamer/GStreamerFormatHelper.cpp
@@ +227,1 @@
>                                   "media.gstreamer.enable-blacklist", true);

How about changing the pref as well?
Attachment #8674980 - Flags: review?(gsquelart) → review+
Priority: -- → P2
Attached patch Rename patch v2 (obsolete) — Splinter Review
Updated patch addressing comments. Carrying forward r=gerald.

s/Blocklist/BlockList/ and s/blocklist/block list/. I left the pref as before to avoid breaking customizations, after discussion with Chris Peterson.
Attachment #8674980 - Attachment is obsolete: true
Attachment #8676521 - Flags: review+
Attached patch Rename patch v3Splinter Review
Fix encoding problem with the first line of the patch. Thanks to gerald for the extra review.
Attachment #8676521 - Attachment is obsolete: true
Attachment #8676530 - Flags: review+
https://hg.mozilla.org/mozilla-central/rev/e129ed7e555e
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla44
You need to log in before you can comment on or make changes to this bug.