Should we make Range a subclass of StaticRange?

[garykac]

From @choniong on May 5, 2016 15:6

According to the StaticRange.idl I found (not sure if that's the latest one) there are some duplicate methods with Range.

See @dtapuska comments below, do we still want to make it interface Range : StaticRange?

BTW would it be better to have the an idl inside this explainer?
Here is the idl garykac/staticrange#2 (comment)

Copied from original issue: garykac/staticrange#1

[garykac]

From @choniong on June 2, 2016 15:6

@dtapuska mentioned that StaticRange should be a subclass of Range, but the idl seems disagree.
@garykac @ojanvafai can we have a confirmation on this question? Thanks!

[garykac]

From @dtapuska on June 2, 2016 15:12

I recall that we were talking about changing Range to be a subclass of StaticRange...

ie: interface StaticRange
and
interface Range : StaticRange

I don't recall what the resolution was in January.

[garykac]

I recall it being mentioned, but there were concerns about changing the definition of Range. I don't recall details or specific concerns, however.

[garykac]

From @choniong on June 3, 2016 20:12

@garykac thanks for the comments!

I'm currently working on StaticRange and is blocked on this issue (https://crrev.com/2022863002).
Can we keep these two separate for now, and maybe changing Range later if we decided to do so?

[masayuki-nakano]

If it's a base of Range, I think that Selection API should change the definition of each methods from Range to StaticRange. Then, web apps doesn't need to create expensive Range object (which may need to add itself to mutation observer) to do something to Selection (e.g., addRange()).

(I'm struggling with Gecko's Range performance issue now...)

[garykac]

If changing Range to be a subclass of StaticRange means that the Selection API could adopt StaticRange (but browsers could still produce Ranges if they wanted to (or during a transition period)), then that sounds like a good reason to make that change.

Are there any risks with changing the definition of Range (just the base class, not any APIs)? I can't think of any offhand.

@choniong @ojanvafai @gked Any thoughts on this?

[ojanvafai]

I think we should change any APIs that take Ranges as an argument to take StaticRanges. Since Range would be subclass of StaticRange, this shouldn't break any existing web content and would remove one more need for authors to create Ranges.

[ojanvafai]

@masayuki-nakano if you're be willing to comment on w3c/input-events#38 (comment) with the performance problems you're seeing, that would be helpful since smaug is unconvinced by my examples that live Ranges are enough of a problem that we should discourage their use and stop creating new APIs that expose live Ranges.

[masayuki-nakano]

@ojanvafai Mainly, the problems which I see depend on implementation. However, if multiple selection ranges will be supported by all web browsers, optimizing only by UA may hit serious performance issue on some complicated web apps in the future. So, if alternative range object is declared, I just think that it should be available everywhere. Then, web app developers can optimize the range performance by themselves.

[garykac]

@masayuki-nakano @smaug---- Could you confirm that you'd be OK with StaticRange being a base class for Range? Or let us know what concerns you have about making that change?
Thanks!

[bzbarsky]

It seems really weird to me to make Range a subclass of StaticRange.

StaticRange is a thing that guarantees that its members will not change as you mutate the DOM. Range guarantees no such thing. If you have an API that takes a StaticRange, it would seem to be reasonable for it to assume that it can arbitrarily order DOM modification and reads of members from the StaticRange. But if Range is a subclass of StaticRange that assumption is not in fact reasonable...

[othermaciej]

@bzbarsky It seems like a similar argument holds for StaticRange inheriting from Range instead, it would not fulfill the API contract. So maybe there needs to be a new base class that they both inherit from.

[masayuki-nakano]

I don't have any concerns. However, it might be better that StaticRange and Range inherits common base class and the base class is used by APIs like @othermaciej said. (Although, I don't have any idea which is better now.)

[rniwa]

It's very unfortunate that we have Range and StaticRange. Had Range being named DynamicRange or LiveRange, we would have had an obvious name for their superclass: Range.

[chaals]

@rniwa

It's very unfortunate that we have Range and StaticRange. Had Range being named DynamicRange or LiveRange, we would have had an obvious name for their superclass: Range.

Yup. But we could always create AbstractRange or something equally ugly. Is there any value in such an abstraction allowing for separate types for contiguous and non-contiguous ranges?

[othermaciej]

AbstractRange would be consistent with AbstractWorker. Another possibility is RangeBase which would be consistent with PaymentDetailsBase and which is a bit shorter. I couldn't find any other examples of either naming pattern.

I agree that in principle, Range as that base class with StaticRange and LiveRange subclasses would be more elegant. It seems like that would probably be ruled out by web compatibility, but maybe not? I don't know how we'd test.

StaticRange might also benefit from having a greater subset of the Range methods and properties. A number of Range methods seem useful independent of the live vs static distinction. Doing the common base class thing would make this relatively easy to accomplish.

[rniwa]

StaticRange might also benefit from having a greater subset of the Range methods and properties.

Yes, in fact, we should add a number of helper methods in Range to StaticRange such as intersectsNode, comparePoint, isPointInRange since otherwise authors would have to create a live Range just to check those conditions with a StaticRange.

[garykac]

How strongly do people feel about having a separate Abstract/Base class for this? To me, StaticRange is just a snapshot that doesn't update, whereas Range is the same thing + it updates with DOM mutations. So it seems reasonable for Range to inherit from StaticRange (other than the unfortunate naming). Although I understand the concerns raised (and I may be to close to the issue, so an outsider's voice is useful here). If we have an Abstract/Base class, then StaticRange would just be exact copy of that class with no additional members or methods (except that you could construct it). That feels a bit odd, but I can live with it. Is that what people prefer?

…

On Mon, Dec 4, 2017 at 12:59 PM, Ryosuke Niwa ***@***.***> wrote: StaticRange might also benefit from having a greater subset of the Range methods and properties. Yes, in fact, we should add a number of helper methods in Range to StaticRange such as intersectsNode, comparePoint, isPointInRange since otherwise authors would have to create a live Range just to check those conditions with a StaticRange. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#1 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABaVSobUmj-_C4whFxI39w1bu92Jr8QBks5s9F0RgaJpZM4NIMue> .

[othermaciej]

Range inheriting from StaticRange would violate the Liskov Substitution Principle, as would doing it the other way around. An abstract base class avoids the LSP violation. Is this the most critical problem? Likely not. But it's nice to make the design more elegant if we can, and its hard to see a reason not to.

We have, I think, four difference inheritance structure possibilities:

1. Range and StaticRange are unrelated interfaces.
2. Range inherits from StaticRange.
3. StaticRange inherits from Range.
4. StaticRange and Range both inherit from AbstractRange (or similar)

Besides the subtyping correctness issue, there are other things to consider:

• More of the methods and properties on Range should also be available on StaticRange (perhaps everything but the constants). Given this, inheritance options 2, 3 and 4 will all lead to an empty or near-empty interface. (The constants in Range probably can't be moved.)
• Options 2, 3 and 4 would all require modifying the DOM spec, or monkey patching it. So if we are agreed to not do 1, the decision among {2, 3, 4} should include the appropriate DOM folks.

[rniwa]

@annevk @domenic per @othermaciej's comment.

[ojanvafai]

My preference would be option 4 from othermaciej's list: create an AbstractRange superclass and move some of the methods on Range that we want on StaticRange to the superclass as well. The specific ones rniwa lists in #1 (comment) make sense to me as a starting point.

[domenic]

We could also use a mixin to ensure the methods and properties are shared even without an inheritance relationship. I'm not at this moment able to think of any advantages or disadvantages either way, but either that or abstract base class seem nice.

[ojanvafai]

In particular, I think there are some things on Range that we don't want on StaticRange. "detach" is a clear example that doesn't make sense for StaticRange. We'd probably want to have a separate discussion about the DOM modification APIs (e.g. extractContents), but my first inclination is that they don't make sense on StaticRange.

[ojanvafai]

domenic: Yup, I also don't have a preference between mixin and abstract base class.

[othermaciej]

Can DOM / Web IDLmethods be specified to take a mixin as a parameter? I think we would want any method that takes a Range to accept AbstractRange.

[bzbarsky]

We could typedef (Range or StaticRange) AbstractRange; if we wanted to. (Note that I'm not necessarily endorsing this, and it's probably somewhat slower than having an actual AbstractRange base interface in at least some implenentations, but it is an option.)

[annevk]

I have a slight preference for a base class as that's more in line with the rest of the DOM.

[domenic]

Base class sounds good to me.

[annevk]

I went ahead and wrote a PR for the DOM Standard that does this given the general lack of activity here (and lack of replies via private email): whatwg/dom#589. A readable diff can be found at https://whatpr.org/dom/589/c05ca60...b3b2950.html#ranges and a version with the changes integrated at https://whatpr.org/dom/589.html#ranges. (The source diff is rather large unfortunately due to the refactoring required.)

I'll write some tests too. Probably best for follow-up comments to go to that PR.