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

Request for TAG review of Input Events level 1 #160

Closed
1 of 3 tasks
johanneswilm opened this issue Mar 20, 2017 · 7 comments
Closed
1 of 3 tasks

Request for TAG review of Input Events level 1 #160

johanneswilm opened this issue Mar 20, 2017 · 7 comments

Comments

@johanneswilm
Copy link

Hello TAG!

I'm requesting a TAG review of:

We have both gone through the security review independently and have not been able to think of any new security issues that do not already exist (such as listening in on users typing passwords in various ways or reccording the typing pattern in order to recognize the user as he/she moves from site to site.)

We'd prefer the TAG provide feedback as (please select one):

  • open issues in our Github repo for each point of feedback
  • open a single issue in our Github repo for the entire review
  • leave review feedback as a comment in this issue and @-notify [github usernames]
@torgo torgo added this to the tag-telcon-2017-03-28 milestone Mar 21, 2017
@triblondon triblondon changed the title Request for TAG review of Input Events spec Request for TAG review of Input Events level 1 Apr 27, 2017
@triblondon
Copy link

Level 2 moved to #173

@cynthia
Copy link
Member

cynthia commented Apr 29, 2017

Thanks a lot for the review request. We are excited to see this work happening - writing editors for the web has always been a painful task, and it's great to see this improving. @travisleithead, @slightlyoff, and I touched on this during our Tokyo F2F.

Here are some first-pass comments:

Just out of curiosity, there is a specific unusual operation that touches transposing the last two characters which were entered - is this a common OS level primitive operation? It would be interesting to know the use case and rationale behind this is. It feels a bit out of place compared to the other generic operations.

This is mostly editorial, but 5.1.2 could be better as a table, the current presentation method feels like it would be defining a sequence algorithm. Grouping operations based on category in the table might make sense in this case.

There are mentions about soft lines and hard lines with no definition, which implies domain knowledge. This isn't always the case, so it would be nice to have it in definition, or link to a definition if it is defined elsewhere.

While we understand that this comes from a underlying infrastructural issue (HTML) - colors are just mentioned as "hex digit value", which could be a bit more specific. I believe the concept you are looking for is simple color (defined in HTML), but this has the downside of not having alpha channel support.

https://html.spec.whatwg.org/multipage/infrastructure.html#colours
http://w3c.github.io/html/infrastructure.html#colors

We discussed briefly within the TAG whether or not CSS typed OM would have something that you could use for some of the style related properties in , this would be worth checking on your side as well.

Adding a link to the explainer document http://w3c.github.io/editing/ from the spec would be great.

Double checking if there are any potential issues with regards to re-entrance in implementations is probably something that we'll need a bit more time on.

@slightlyoff had concerns about beforeinput not firing on input elements. Further comments should come from his direction.

Additionally, we looked at both level 2 and level 1, and the different pieces in level 2 seemed to be fine for us, so unless there are specific parts that you would like us to look into specific to level 2, we could close #173 and fold it back into this issue.

@cynthia cynthia added Progress: pending external feedback The TAG is waiting on response to comments/questions asked by the TAG during the review and removed Progress: in progress labels Apr 29, 2017
@slightlyoff
Copy link
Member

slightlyoff commented Apr 29, 2017

Re: onbeforeinput for <input>, <textarea>, etc.

In the past I've had to write hacky code to fixing up copy/paste/drop/etc. in input types, not just for contenteditable elements. One can argue that the oninput and onchange events allow everything one might need, but for the same reasons that it's difficult to fixup content in contenteditable, it's hard in <input> and friends. It seems natural for them to also have onbeforeinput. Similarly, it seems reasonable for oninput and onchange to be fired on contenteditable elements.

All of this is unsatisfying because it's a patch for forms and input elements is to break out (and make pluggable) the implicit controllers, models, and views that sit behind all of our input types. It's important for us to get these events exposed while that larger architectural issue is sorted out, tho.

@johanneswilm
Copy link
Author

johanneswilm commented Apr 29, 2017

@slightlyoff Indeed, however this should all also apply to <textarea> and <input type="text"> [1]. The problem may be here that this is only mentioned in a non-normative part of the spec and the UI events spec does not seem to mention it at all [2]. I guess this should also be enabled for <input> elements with the type ste to one of email, password, number, search. Any others?

beforeinput and input should be coupled as specified in uievents [2], so that in this order:

  1. beforeinput
  2. DOM change (unless beforeinput was canceled)
  3. input

At what point would you suggest to add the change event?

[1] https://w3c.github.io/input-events/#h-overview

[2] https://www.w3.org/TR/uievents/#interface-inputevent

@johanneswilm
Copy link
Author

@cynthia Thanks for the detailed feedback!

Just out of curiosity, there is a specific unusual operation that touches transposing the last two characters which were entered - is this a common OS level primitive operation? It would be interesting to know the use case and rationale behind this is. It feels a bit out of place compared to the other generic operations.

@cynthia We investigated what editing operations are supported by browsers already. We then tried to be consistent in what operations we added [1]. For these first versions of the spec we did not add new editing operations not found in any of the major browsers on any platfform. The transpose example mentioned here is supported in Chrome and Safari on MacOS [2]. Whether this operation really is used and/or useful is a debate that we have chosen not to engage in for the moment.

The points mention all seem to make sense. I agree that there doesn't need to be a second issue for level 2 as the difference are really minor (the main difference is level 1 doesn't guarantee that a larger number of beforeinput types are cancelable and that level 2 guarantees more input before/input events during IME composition).

[1] For example: if there is a way to delete from caret position to the end of the paragraph, there probably should also be one to delete from caret position to the start of the paragraph.

[2] w3c/editing#148 (comment)

@johanneswilm
Copy link
Author

Hey,
I have made the changes as requested. In the case of input/textarea, the language should now be clearer specifying that it also applies to these, but ultimately, this needs to be added/changed in the ui-events spec, which is where the basic parts of the beforeinput/input events are described. I have posted an issue with them (see above).

I have converted lists to tables, as requested and made descriptions more useful also for those wiothout domain knowledge (hopefully), by avoiding circular descriptions of hardlines and softlines.

One thing that didn't seem possible was to combine several items in the one particular table, as it describes the meaning of the various inputType values.

@triblondon
Copy link

Thanks for making these changes. We're happy with this and will focus our next review on level 2. See #173 (comment).

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

6 participants