Closed Bug 984259 Opened 6 years ago Closed 5 years ago

Code clean up - remove some code about HTMLMediaElement preload

Categories

(Core :: Audio/Video, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla35
Tracking Status
firefox34 --- fixed
firefox35 --- fixed

People

(Reporter: jwwang, Assigned: jwwang)

References

Details

Attachments

(1 file)

User Agent: Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Ubuntu Chromium/32.0.1700.102 Chrome/32.0.1700.102 Safari/537.36
https://bugzilla.mozilla.org/show_bug.cgi?id=886188#c82
> In test 12:
> 
> // 12. Change preload value from auto to metadata after load started,
> // should still do full load, should not halt after metadata only.
> 
> I can't find such a statement in the spec:
> http://dev.w3.org/html5/spec-preview/media-elements.html.
> In our current implementation, the later preload value will overwrite the
> previous one no matter whether load is started. In test 12, preload is
> changed to 'metadata' which tells the user agent just to download enough
> data to decode metadata. Therefore, 'canplaythrough' won't be fired since
> the user agent stops download after it has enough data to decode metadata
> and it results in timeout of the test.

Since the test will be disabled, we should also remove the related code that is not used anymore.
OS: Linux → All
Hardware: x86_64 → All
Depends on: 886188
Blocks: 1041363
http://dev.w3.org/html5/spec-preview/media-elements.html#dom-media-preload

Now the spec. says:
"The preload attribute is an enumerated attribute. The following table lists the keywords and states for the attribute — the keywords in the left column map to the states in the cell in the second column on the same row as the keyword. The attribute can be changed even once the media resource is being buffered or played; the descriptions in the table below are to be interpreted with that in mind."

We should remove the don't-preload-less-when-loading-resource rule.
Assignee: nobody → jwwang
Status: NEW → ASSIGNED
Attachment #8499372 - Flags: review?(cpearce)
No longer depends on: 886188
Attachment #8499372 - Flags: review?(cpearce) → review+
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/4de43ef387d2
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla35
Comment on attachment 8499372 [details] [diff] [review]
984259_remove_code_not_conforming_to_spec.patch

Approval Request Comment
[Feature/regressing bug #]:unknown
[User impact if declined]:To fix the test failure in bug 1041363 which depends on bug 984259.
[Describe test coverage new/current, TBPL]:
Try: https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=ecf19915ab11
[Risks and why]: Low. The patch is simple and has been run on Central for a while.
[String/UUID change made/needed]:none
Attachment #8499372 - Flags: approval-mozilla-aurora?
Comment on attachment 8499372 [details] [diff] [review]
984259_remove_code_not_conforming_to_spec.patch

OK. Small fix. Let's take this on Aurora to clean up the intermittent test failure.
Attachment #8499372 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Flags: qe-verify-
You need to log in before you can comment on or make changes to this bug.