Closed Bug 1329564 Opened 3 years ago Closed 3 years ago

StringListRange (in dom/media/VideoUtils.h) should have options on how to handle empty items

Categories

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

49 Branch
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla53
Tracking Status
firefox53 --- fixed

People

(Reporter: gerald, Assigned: gerald)

References

Details

Attachments

(2 files)

Currently StringListRange always skips empty items. So "a,,b" will only go through 'a' and 'b', but not the empty element.

For some upcoming work, handling empty items will be important, so I will add an option to do that.

Special consideration is needed for a totally-empty string "": Is it an empty list, or a list of one empty item? Different options will be offered for both situations.
Blocks: 1329568
Comment on attachment 8824871 [details]
Bug 1329564 - Option to process empty items in StringListRange -

https://reviewboard.mozilla.org/r/103186/#review103860

::: dom/media/VideoUtils.h:369
(Diff revision 1)
>  
> -template <typename String>
> +enum class StringListRangeEmptyItems
> +{
> +  Skip, // Skip all empty items (empty string will process nothing)
> +  ProcessAll, // Process all, including 1 empty item in an empty string
> +  ProcessEmptyItems // Process all, except if string is empty

I don't follow the name or comment here.

So it process Empty Items except if string is empty???
Attachment #8824871 - Flags: review?(jyavenard) → review+
Comment on attachment 8824872 [details]
Bug 1329564 - StringListRange::begin/end() can be const -

https://reviewboard.mozilla.org/r/103188/#review103864
Attachment #8824872 - Flags: review?(jyavenard) → review+
Comment on attachment 8824871 [details]
Bug 1329564 - Option to process empty items in StringListRange -

https://reviewboard.mozilla.org/r/103186/#review103860

> I don't follow the name or comment here.
> 
> So it process Empty Items except if string is empty???

I don't follow your question! You seem to understand, by repeating what the comment says. :-)

The two will process empty items, e.g.: "a,,b" will give "a", "", "b". And "," will give "", "" (two empty items).
The main difference is how they handle an empty string, because users may have different expectations:
- ProcessAll: "" -> one item, which is just ""
- ProcessEmptyItems: "" -> Zero items, nothing will happen.

How could I phrase it better, so it's as clear as possible? I probably could give an example, as above.
Pushed by gsquelart@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/195a58e1ad5e
Option to process empty items in StringListRange - r=jya
https://hg.mozilla.org/integration/autoland/rev/d3da2d43ac5d
StringListRange::begin/end() can be const - r=jya
https://hg.mozilla.org/mozilla-central/rev/195a58e1ad5e
https://hg.mozilla.org/mozilla-central/rev/d3da2d43ac5d
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla53
You need to log in before you can comment on or make changes to this bug.