This is an archived snapshot of W3C's public bugzilla bug tracker, decommissioned in April 2019. Please see the home page for more details.

Bug 22471 - "buffered" attribute returns a new object each time
Summary: "buffered" attribute returns a new object each time
Status: RESOLVED MOVED
Alias: None
Product: WHATWG
Classification: Unclassified
Component: HTML (show other bugs)
Version: unspecified
Hardware: Other All
: P3 normal
Target Milestone: Unsorted
Assignee: Anne
QA Contact: contributor
URL: http://www.whatwg.org/specs/web-apps/...
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-06-26 05:37 UTC by contributor
Modified: 2017-08-02 07:17 UTC (History)
8 users (show)

See Also:


Attachments

Description contributor 2013-06-26 05:37:44 UTC
Specification: http://www.whatwg.org/specs/web-apps/current-work/multipage/the-video-element.html
Multipage: http://www.whatwg.org/C#loading-the-media-resource
Complete: http://www.whatwg.org/c#loading-the-media-resource
Referrer: http://www.whatwg.org/specs/web-apps/current-work/multipage/

Comment:
"buffered" attribute returns a new object each time

Posted from: 98.110.194.206 by bzbarsky@mit.edu
User agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.8; rv:24.0) Gecko/20130606 Firefox/24.0
Comment 1 Boris Zbarsky 2013-06-26 05:38:59 UTC
Returning a new object each time from an attribute getter (as opposed to a method) is terrible API, because it breaks the very reasonable expectation that "foo.bar == foo.bar".

We should stop doing this, if we can.  If we can't, we should deprecate these attributes and replace them with methods...
Comment 2 Boris Zbarsky 2013-06-26 05:40:41 UTC
"played" and "seekable" have the same issue.

One other note: even if we don't plan to change these attributes we should at least add a note on them saying they're bad API, since people are cargo-culting them in other specs....
Comment 3 Ian 'Hixie' Hickson 2013-07-02 23:34:51 UTC
What would you like the note to say, exactly, and where should I put it?
Comment 4 Boris Zbarsky 2013-07-02 23:49:08 UTC
Next to the prose that says that this attribute returns a new object each time, and it should say something like:

  This is a terrible API that should not be cargo-culted.  If you want to return
  a new object each time, use a method, not an attribute.
Comment 5 Ian 'Hixie' Hickson 2013-07-03 22:23:17 UTC
That would not set a good precedent — there's tons of other things in the Web API that we could say that about, and I don't want the HTML spec to become a treatise in the failings of the Web. It's already trying to wear too many hats.
Comment 6 Ian 'Hixie' Hickson 2013-07-03 22:25:41 UTC
I'd put it in a <!-- comment -->, but I don't think most spec writers are reading the spec source when they copy an API. Frankly, there's a good chance that they're not even copying the spec, just APIs as they find them — should we add this warning to all the MDN pages? To the JS console?

I think maybe we would be better off solving this problem by having people review specs more, and by having implementors refuse to implement stuff like this.
Comment 7 Boris Zbarsky 2013-07-04 03:19:14 UTC
> I think maybe we would be better off solving this problem by having people
> review specs more

That's how I came across the problem, yes.  I was asked for feedback on a patch that was implementing a spec that had cargo-culted this pattern from .buffered.

> Frankly, there's a good chance that they're not even copying the spec

The spec author explicitly said they had copied what the .buffered spec says, when I asked them why they had set it up that way....
Comment 8 Ian 'Hixie' Hickson 2013-07-09 21:07:18 UTC
Sure, that particular one, but maybe the next one copies seekable, and another copies some other spec, and another copies MDN, etc.

There's tons of bad APIs in the Web. I don't think it is in the overall interests of the Web for us to keep drawing attention to this. It doesn't scale well, it makes newcomers who want to learn the Web get discouraged more quickly than they otherwise would, it gives detractors fodder for trolling, it's just not a long-term workable plan, IMHO. See also comment 5.

Did you happen to ask them if they looked at the spec source to cargo-cult this?

Do you know if they looked at the W3C copy or the WHATWG copy?

Did they read any of the introductory material? I could put something there, if people would read it

Did they click on this part of the spec e.g. to copy-and-paste it? I could make something appear on mouse down or something like that... maybe a warning icon in the margin, similar to the fingerprinting thing? The question is, would they have followed it to find out what it said?

Can you have said spec editor come to this bug and comment on the above?
Comment 9 Boris Zbarsky 2013-07-09 21:58:45 UTC
> Can you have said spec editor come to this bug and comment on the above?

Let's try that.
Comment 10 Aaron Colwell 2013-07-09 22:53:50 UTC
I don't particularly care for the cargo-culting charactarization since it implies that I didn't actually think about this. 

The reason I copied buffered to match the HTMLMediaElement attribute is because the result of the SourceBuffer.buffered attribute is a subset of what HTMLMediaElement.buffered reports. It seemed natural to make this functionality look and act the same way as the HTMLMediaElement version since people familiar with the one would automatically be familiar with the behavior of the other. 

I don't think it is particularly useful to get so worked up about this. I did bring up this issue on our MSE spec teleconference call today and others favored keeping the spec as is so that it matches the HTMLMediaElement. I changed the other attribute you objected to in the Media Source spec since your argument made sense for that and there weren't any other reasons to prefer the attribute construct over a method.

I don't believe there is much to gain by deprecating these attributes in favor of methods. It seems like it would just break sites for no apparent gain other than "API purity". I'm just trying to be pragmatic here. If a new set of TimeRanges are added to HTMLMediaElement, then yes by all means we should use methods, but I don't think it is worth trying to convert the existing TimeRanges attributes that are already there. In the grand scheme of things this seems like a pretty minor bit of legacy to keep around.
Comment 11 Ian 'Hixie' Hickson 2013-07-10 00:16:14 UTC
Thanks Aaron.

bz: I think, based on the above, that a comment in the spec, whether in the source, or visible to all, or in the introduction section, would actually have had no effect at all. So I don't know that it's worth adding anything here.
Comment 12 Boris Zbarsky 2013-07-10 02:04:57 UTC
I'm sorry; I did not mean to imply that you didn't think when you copied the pattern.

But, imo, that makes it even more important to point out that the pattern should not be copied in new APIs...  It's a _really_ bad pattern from the point of view of JS programmers, because it badly violates basic expectations about how attribute gets should behave.
Comment 13 Aaron Colwell 2013-07-10 03:43:47 UTC
(In reply to comment #12)
> I'm sorry; I did not mean to imply that you didn't think when you copied the
> pattern.

Thanks. I appreciate this.

> 
> But, imo, that makes it even more important to point out that the pattern
> should not be copied in new APIs...  It's a _really_ bad pattern from the
> point of view of JS programmers, because it badly violates basic
> expectations about how attribute gets should behave.

I understand, but I don't think the HTML spec itself is the best place to do this.

It would be nice if there was a "Best Practices" or "Web API Design" document that spec authors could refer to so they could learn from the mistakes of the past and avoid introducing new APIs with the same old mistakes. It seems like such a document would be a better place for do's and don'ts almost like the coding style guides we use for Chromium (http://www.chromium.org/developers/coding-style , http://google-styleguide.googlecode.com/svn/trunk/cppguide.xml). I know that such a document would have been enormously helpful when I started writing the MSE spec. It is hard to know when to emulate patterns from the past and when to just adopt the new hotness even if it doesn't match anything that surrounds it.
Comment 14 Ian 'Hixie' Hickson 2013-07-23 22:53:35 UTC
I've added something about this here:

   http://wiki.whatwg.org/wiki/Howto_spec#Defining_an_attribute

Feel free to add more such things on that page.
Comment 15 Boris Zbarsky 2015-01-09 12:52:49 UTC
This is not fixed.  You can call it wontfix if you want, but the basic brokenness of the API is still there; the bit added in comment 14 certainly doesn't fix that.
Comment 16 Boris Zbarsky 2015-01-09 12:53:23 UTC
> You can call it wontfix if you want

Though obviously it would be better to fix the API....
Comment 17 Ian 'Hixie' Hickson 2015-01-09 23:36:43 UTC
It seems unlikely that we can make a breaking change to this API. What change would you suggest?
Comment 18 Boris Zbarsky 2015-01-10 03:01:38 UTC
At what point can the set of time ranges actually change?

It it can't change during JS execution that doesn't call into DOM methods, then the simplest thing is to keep returning the same value until the set of time ranges changes, then create a new object with the new set of time ranges.

That way the behavior will fundamentally be the same as that of the .firstChild property: it keeps reporting the same value until you do something that changes it.
Comment 19 Philip Jägenstedt 2015-01-13 19:11:07 UTC
(In reply to Boris Zbarsky from comment #18)
> At what point can the set of time ranges actually change?
> 
> It it can't change during JS execution that doesn't call into DOM methods,
> then the simplest thing is to keep returning the same value until the set of
> time ranges changes, then create a new object with the new set of time
> ranges.
> 
> That way the behavior will fundamentally be the same as that of the
> .firstChild property: it keeps reporting the same value until you do
> something that changes it.

Aaron is OK with this model in bug 27790 and I agree. Hixie, do you think this is model is acceptable, so that we can do the same in HTML and MSE?
Comment 20 Ian 'Hixie' Hickson 2015-01-13 23:18:09 UTC
We can change the API if you don't think it'll break usage, but it seems unlikely to me that it won't break usage.

Having said that, it seems to me to be a weird change. I don't really understand what's wrong with the current API, but it seems that sometimes returning the same object and sometimes not is even more weird than always returning a new object. Especially for something which conceptually changes every nanosecond (I don't recall off-hand if this particular API is actually being frozen per task).
Comment 21 Boris Zbarsky 2015-01-14 02:19:07 UTC
> but it seems unlikely to me that it won't break usage.

What do you think it would break?

> it seems that sometimes returning the same object and sometimes not is even
> more weird than always returning a new object.

No, it's not.  It's no more weird than .firstChild, as I said.  The key is that this behavior makes perfect sense if the value can only change when you give up control to some code that changes it.  That's quite different from "sometimes returning the same object and sometimes not" in that it's fairly predictable in terms of when it can change: when the event loop runs or when you explicitly invoke some method or setter that changes the value.  If you're just running script to completion and don't modify the value yourself, you know it won't change.

> Especially for something which conceptually changes every nanosecond

This would be a bigger issue.  If that's really the behavior, this should _really_ never have been an attribute....  But yes, if the expected behavior here is more like that of performance.now(), then updating only on event loops spins or explicit changes is clearly wrong.  Hence the question I asked in comment 18.
Comment 22 Philip Jägenstedt 2015-01-14 09:19:12 UTC
(In reply to Ian 'Hixie' Hickson from comment #20)
> Especially for something which conceptually changes
> every nanosecond (I don't recall off-hand if this particular API is actually
> being frozen per task).

currentTime uses "The official playback position is an approximation of the current playback position that is kept stable while scripts are running."

I think buffered should be updated to say something similar, it currently says "The buffered attribute must return a new static normalised TimeRanges object that represents the intersection of the ranges of the media resources of the slaved media elements that the user agent has buffered, at the time the attribute is evaluated."

More generally, I think that the whole state of HTMLMediaElement should be consistent and unchanging while a script is running.
Comment 23 Ian 'Hixie' Hickson 2015-01-15 19:25:06 UTC
IMHO the problem with this API is not that it can change while script is running, nor that it returns a new object each time you fetch it, it's just that it uses an attribute for an expensive operation, and attributes are an affordance that appears cheap, so it misleads authors regarding its cost.

I really don't see why an attribute can't change value every time you fetch it. Nor do I see why it's bad for the value to change while script is running.

Testing this, it seems like Firefox, Chrome, and Safari all return new objects each time but don't update the values while script is running:
   http://software.hixie.ch/utilities/js/live-dom-viewer/?saved=3372

So I guess we could update the spec to say that the value returned is that at the time the event loop last reached step 1. Changing the attribute's allocation behaviour seems unwise given the interop though.
Comment 24 Boris Zbarsky 2015-01-15 19:43:10 UTC
> it's just that it uses an attribute for an expensive operation

Yes, that's an issue too.  But it's less of an issue if the value gets cached for after the first get...

> I really don't see why an attribute can't change value every time you fetch it.

It _can_, clearly; getters let you do that.

But it _shouldn't_, because it really confuses authors (in addition to making the operation more expensive in this case).  People have been saying this for years.

I would really like to change the Firefox behavior here, for what it's worth.  I don't expect it to be a compat problem.
Comment 25 Anne 2016-03-09 14:26:38 UTC
This seems like something we should change, indeed. Philip, can you work on this?
Comment 26 Philip Jägenstedt 2016-03-15 08:30:38 UTC
OK, I took a quick peek, but MediaController also has TimeRanges attributes and I don't want to spend any time on that. I suggest revisiting once MediaController has been removed: https://github.com/whatwg/html/issues/192

An implementor interested in fixing this and writing web-platform-tests in the process would also be good. The way this currently works in Blink makes it very tempting to cheat by only returning a new object if the ranges have changed since last accessed, but that wouldn't match what I intend for the spec to say. If the buffered ranges change and then change back (most easily in the case of no ranges) then it should still be a new object, if it *could* have been observed but just wasn't.
Comment 27 Anne 2017-07-21 09:38:04 UTC
MediaController is gone now. I assume you're still interested in working on this Philip?
Comment 28 Philip Jägenstedt 2017-08-01 08:53:08 UTC
I'm OK with being the assignee, but am no longer so sure we should attempt to change this. https://jsbin.com/nugiloq/edit?html,output is a test that passes in Chrome, Edge, Firefox and Safari, and fixing this would mean moving away from that interoperable state. I can't see a way to estimate the risk with use counters either, because it depends on how the returned objects are used.

So, I think we should go the path of adding tests for this (if they don't already exist) and adding comments in the spec that this behavior is weird and not to be copied.
Comment 29 Boris Zbarsky 2017-08-01 13:26:26 UTC
Notes, not comments.  They need to be visible to spec readers, not spec authors.
Comment 30 Philip Jägenstedt 2017-08-01 13:40:03 UTC
Yes, that's what I meant but didn't write :)
Comment 31 Domenic Denicola 2017-08-01 14:58:08 UTC
Do you anticipate this causing compat problems to change? We've changed several similar things in the past...
Comment 32 Philip Jägenstedt 2017-08-01 15:20:41 UTC
I don't have a specific piece of breaking code that I suspect is out there, and it seems fairly likely that such a change would be web compatible. I suppose I'm just not feeling super motivated to spend time on it given that at best nobody will notice, but maybe I should mark it available instead?
Comment 33 Anne 2017-08-02 07:17:37 UTC
https://github.com/whatwg/html/pull/2884

If someone wants to put effort into changing it, I recommend we do that through a new set of issues.