Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Give StaticRange a constructor #5

Closed
foolip opened this issue Apr 10, 2017 · 9 comments
Closed

Give StaticRange a constructor #5

foolip opened this issue Apr 10, 2017 · 9 comments

Comments

@foolip
Copy link

foolip commented Apr 10, 2017

It's now immutable, but could still have a constructor:

dictionary StaticRangeInit {
  required Node startContainer;
  required unsigned long startOffset;
  required Node endContainer;
  required unsigned long endOffset;
};

[Constructor(StaticRangeInit initDict)]
interface StaticRange { ... };

If there's no use for invalid ranges, then it could throw if http://garykac.github.io/staticrange/index.html#to-is-valid does not hold.

@chong-z
Copy link

chong-z commented Apr 11, 2017

Do we have a resolution on what should happen if the range is invalid? IIUC the previous version of spec wants to do some normalization, e.g. It will tweak start and end to make the range valid.

@garykac Can you confirm the expected behavior here?

If there is no clear resolution or requires further discussion would it be better to ship StaticRange without a constructor or with only an empty constructor first?

@garykac
Copy link
Owner

garykac commented Apr 11, 2017

@foolip What's the rule-of-thumb for adding constructors?

AIUI, DataTransfer originally didn't have one because there was no use case for it. That seems similar to the situation with StaticRange: We don't need one now, but we could add one later when there is a need.

Or should we default to adding a constructor even when there is no API need for one? I can't think of a use for it.

@choniong If we decide to add a constructor, then we would need to specify either (1) throw, (2) normalize like Ranges do. My pref is (2), but I don't think we need a constuctor.

@foolip
Copy link
Author

foolip commented Apr 13, 2017

The rule-of-thumb that I have inherited from @domenic and @slightlyoff is that things should have a constructor by default unless the objects are somehow "exotic" or cannot be created synchronously. ImageBitmap is an example where there are good reasons I think, but I'm not in touch with the details.

Here, I guess that the constructor should simply copy over the values from the init dictionary. Any StaticRange that the UA creates can be made invalid by modifying the DOM enough, so I don't think trying to prevent scripts from creating initially-invalid ranges would help.

@domenic
Copy link

domenic commented Apr 13, 2017

Right. In general we are trying to minimize the amount of magic on the platform. Objects that the UA is magically able to construct somehow, despite the class in question not having a constructor, are serious magic. In some cases (e.g. ImageBitmap) that magic is necessary, but definitely not here.

@foolip
Copy link
Author

foolip commented Apr 13, 2017

@whsieh @rniwa, any concerns about adding a constructor in WebKit?

@rniwa
Copy link

rniwa commented Apr 13, 2017

Please just throw when the range is invalid then I don't see any reason adding a constructor is problematic.

@foolip
Copy link
Author

foolip commented Apr 17, 2017

I suppose throwing might catch some mistakes, so that seems fine to me, even if the range can later become invalid. I defer to @choniong and @garykac to say what they'd prefer.

@garykac
Copy link
Owner

garykac commented Apr 17, 2017 via email

@garykac
Copy link
Owner

garykac commented Apr 25, 2017

This issue was moved to w3c/staticrange#3

@garykac garykac closed this as completed Apr 25, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants