22471 2013-06-26 05:37:44 +0000 "buffered" attribute returns a new object each time 2017-08-02 07:17:37 +0000 1 1 1 Unclassified WHATWG HTML unspecified Other All RESOLVED MOVED http://www.whatwg.org/specs/web-apps/current-work/#loading-the-media-resource P3 normal Unsorted 1 contributor annevk acolwell annevk bzbarsky d ian kinetik mike philipj contributor oldest_to_newest 89870 0 contributor 2013-06-26 05:37:44 +0000 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 89871 1 bzbarsky 2013-06-26 05:38:59 +0000 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... 89872 2 bzbarsky 2013-06-26 05:40:41 +0000 "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.... 90184 3 ian 2013-07-02 23:34:51 +0000 What would you like the note to say, exactly, and where should I put it? 90194 4 bzbarsky 2013-07-02 23:49:08 +0000 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. 90275 5 ian 2013-07-03 22:23:17 +0000 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. 90276 6 ian 2013-07-03 22:25:41 +0000 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. 90321 7 bzbarsky 2013-07-04 03:19:14 +0000 > 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.... 90533 8 ian 2013-07-09 21:07:18 +0000 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? 90545 9 bzbarsky 2013-07-09 21:58:45 +0000 > Can you have said spec editor come to this bug and comment on the above? Let's try that. 90553 10 acolwell 2013-07-09 22:53:50 +0000 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. 90566 11 ian 2013-07-10 00:16:14 +0000 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. 90575 12 bzbarsky 2013-07-10 02:04:57 +0000 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. 90578 13 acolwell 2013-07-10 03:43:47 +0000 (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. 91166 14 ian 2013-07-23 22:53:35 +0000 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. 116959 15 bzbarsky 2015-01-09 12:52:49 +0000 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. 116960 16 bzbarsky 2015-01-09 12:53:23 +0000 > You can call it wontfix if you want Though obviously it would be better to fix the API.... 116980 17 ian 2015-01-09 23:36:43 +0000 It seems unlikely that we can make a breaking change to this API. What change would you suggest? 116989 18 bzbarsky 2015-01-10 03:01:38 +0000 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. 117093 19 philipj 2015-01-13 19:11:07 +0000 (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? 117114 20 ian 2015-01-13 23:18:09 +0000 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). 117130 21 bzbarsky 2015-01-14 02:19:07 +0000 > 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. 117143 22 philipj 2015-01-14 09:19:12 +0000 (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. 117223 23 ian 2015-01-15 19:25:06 +0000 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. 117229 24 bzbarsky 2015-01-15 19:43:10 +0000 > 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. 125403 25 annevk 2016-03-09 14:26:38 +0000 This seems like something we should change, indeed. Philip, can you work on this? 125496 26 philipj 2016-03-15 08:30:38 +0000 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. 128732 27 annevk 2017-07-21 09:38:04 +0000 MediaController is gone now. I assume you're still interested in working on this Philip? 128815 28 philip 2017-08-01 08:53:08 +0000 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. 128816 29 bzbarsky 2017-08-01 13:26:26 +0000 Notes, not comments. They need to be visible to spec readers, not spec authors. 128817 30 philip 2017-08-01 13:40:03 +0000 Yes, that's what I meant but didn't write :) 128818 31 d 2017-08-01 14:58:08 +0000 Do you anticipate this causing compat problems to change? We've changed several similar things in the past... 128819 32 philip 2017-08-01 15:20:41 +0000 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? 128820 33 annevk 2017-08-02 07:17:37 +0000 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.