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.

Sign up for GitHub

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

Jump to bottom

Declare more detail of data value of formatBackColor and formatFontColor #94

Closed
masayuki-nakano opened this issue Feb 6, 2019 · 12 comments
Closed

Declare more detail of data value of formatBackColor and formatFontColor #94

masayuki-nakano opened this issue Feb 6, 2019 · 12 comments

Comments

@masayuki-nakano
Copy link

Currently, no browser sets data attribute to "RGB function call" declared by Input Events even when inputType is either formatBackColor or formatFontColor.

Additionally, these inputType are used only when execCommand is used. Finally, the definition, "RGB function call" is too unclear, for example, rgb() vs. rgba, using int vs. percentage to each component, etc.

So, I think that if we don't have any reasonable reasons to set data attribute for them, null is better value.

Otherwise, please declare the detail of "RGB function call" for compatibility between browsers.
@whsieh
Copy link

whsieh commented Feb 6, 2019 •
edited
Loading

Safari on macOS sets the data attribute of input/beforeinput events when using the Touch Bar to change font color. Currently, this data is of the format "rgb(x, y, z)".

@masayuki-nakano
Copy link
Author

Safari on macOS sets the data attribute of input/beforeinput events when using the Touch Bar to change font color. Currently, this data is of the format "rgb(x, y, z)".

Thanks, then, Firefox will follow it. On the other hand, the unclear definition is a problem for me. Input Events should declare browses should use whether rgb or rgba, should use whether number or percentage for each component, should insert a whitespace before each component or not.

@masayuki-nakano
Copy link
Author

(I like rgba for the forward compatibility, integer for avoiding struggling with float percentage values, and inserting a whitespace before component for easier to read.)

@masayuki-nakano masayuki-nakano changed the title Should data value of formatBackColor and formatFontColor be null? Declare more detail of data value of formatBackColor and formatFontColor Feb 7, 2019
@masayuki-nakano
Copy link
Author

And if initial, inherit, currentColor or transparent are specified by execCommand, what value should be set to? The first 3 require to access style system...

I think that when browser sets color, rgba function (or rgb function) should be used, but data value of input event caused by execCommand, setting specified value as-is is reasonable.

@rniwa
Copy link

rniwa commented Feb 7, 2019

We should probably just follow what CSS OM does, which switches between rgb(~) and rgba(~) based on the alpha value: https://drafts.csswg.org/cssom/#serialize-a-css-component-value

@masayuki-nakano
Copy link
Author

We should probably just follow what CSS OM does, which switches between rgb(~) and rgba(~) based on the alpha value: https://drafts.csswg.org/cssom/#serialize-a-css-component-value

Makes sense!

@masayuki-nakano
Copy link
Author

masayuki-nakano commented Feb 10, 2019 •
edited
Loading

There are two problems:

  1. It requires more cost to implement/compute serialized color value if execCommand is called with currentColor (Level 3, in Level 4, declared as currentcolor), inherit or initial.
  2. If given value of evecCommand, cannot be computed good serialized value.

I suggest that in those cases, given value should be set to data as-is. If CSS color value will be extended in the future, even older browsers can run with any values.

@rniwa
Copy link

rniwa commented Feb 13, 2019

Do browsers even support invoking execCommand with currentColor?

@masayuki-nakano
Copy link
Author

Do browsers even support invoking execCommand with currentColor?

Firefox works as usual colors, e.g., if you select some text and call execCommand("backColor", false, "currentColor"), Firefox wraps it with <span style="background-color: currentcolor"> and </span>.

Edge works similarly. But using <font> element and does not fire input event.

Blink does nothing except dispatching input event with empty inputType.

moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this issue Feb 19, 2019
@masayuki-nakano
Bug 998941 - part 1-7: Make HTMLEditor set InputEvent.data to seriali…
551de12 
…zed color value when InputEvent.inputType is "formatBackColor" or "formatForeColor" r=smaug,m_kato,emilio

Although neither Chrome nor Safari does not set InputEvent.data when the event
is caused by `document.execCommand()` with `backColor`, `foreColor` nor
`hiliteColor`, Safari supports styling color with touchbar and in that case,
Safari sets it (*1).

Additionally, currently Safari uses `rgb()` to represents a color value and
using same rule to serializing color value for CSS OM matches Safari's behavior
and can represent any valid color values.

This patch makes given color value parsed and then serialized with same code
in style system.  If the value is `currentcolor`, `inherit`, `initial` or `reset`, sets
the value as-is for now.  Additionally, when given value is invalid, sets the value
as-is for forward compatibility.

Note that automated tests will be added into input-events-exec-command.html
by the last patch.

1. w3c/input-events#94 (comment)

Differential Revision: https://phabricator.services.mozilla.com/D19295

--HG--
extra : moz-landing-system : lando
mykmelez pushed a commit to mykmelez/gecko that referenced this issue Feb 19, 2019
@masayuki-nakano
Bug 998941 - part 1-7: Make HTMLEditor set InputEvent.data to seriali…
9881713 
…zed color value when InputEvent.inputType is "formatBackColor" or "formatForeColor" r=smaug,m_kato,emilio

Although neither Chrome nor Safari does not set InputEvent.data when the event
is caused by `document.execCommand()` with `backColor`, `foreColor` nor
`hiliteColor`, Safari supports styling color with touchbar and in that case,
Safari sets it (*1).

Additionally, currently Safari uses `rgb()` to represents a color value and
using same rule to serializing color value for CSS OM matches Safari's behavior
and can represent any valid color values.

This patch makes given color value parsed and then serialized with same code
in style system.  If the value is `currentcolor`, `inherit`, `initial` or `reset`, sets
the value as-is for now.  Additionally, when given value is invalid, sets the value
as-is for forward compatibility.

Note that automated tests will be added into input-events-exec-command.html
by the last patch.

1. w3c/input-events#94 (comment)

Differential Revision: https://phabricator.services.mozilla.com/D19295
@masayuki-nakano
Copy link
Author

FYI: I implemented InputEvent.data on Firefox (shipped in 67 if we don't get any "break the web" reports). At that time, we use same serialization of CSS computed style (as @rniwa -san said in comment). And we use given values as-is if we the given value cannot be represented with rgb() nor rgba(). For example, inherit, currentcolor.

@johanneswilm
Copy link
Contributor

I don't remember how I arrived at the wording "RGB function call", but searching the internet a bit, it basically covers rgb() and rgba(). I agree with @rniwa that we should follow CSS OM, and it seems everyone else does too, so I will update accordingly.

@masayuki-nakano Are you saying that you will accept something other than "rgb()" and "rgba()"? As far as I know, the only place this shows up are special menus on Safari. Will Firefox also implement those menus? Will Firefox then have a menu where you can switch the color of a text to "inherit"?

execCommand is not covered by this spec - things related to execCommand should be discussed at https://github.com/w3c/editing . Although that spec has a big disclaimer saying that it's likely never getting finished and browser don't follow it, if Firefox has decided to change course and work more on execCommand, that's where that should go, and we can make a special section about input event related things in there as well.

johanneswilm added a commit that referenced this issue Mar 28, 2019
@johanneswilm
use cssom color serialization, fixes #94 + respec fixes
148af23
@masayuki-nakano
Copy link
Author

@masayuki-nakano Are you saying that you will accept something other than "rgb()" and "rgba()"? As far as I know, the only place this shows up are special menus on Safari. Will Firefox also implement those menus? Will Firefox then have a menu where you can switch the color of a text to "inherit"?

No, Firefox does not have such interface. However, execCommand can cause firing input event with data value having color value. I think that if execCommand does not allow to use inherit etc as the color value, Firefox would stop handling such value (currently, Firefox just sets color or background-color property to coming value even if it's invalid value).

gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified that referenced this issue Oct 3, 2019
@marco-c
Bug 998941 - part 1-7: Make HTMLEditor set InputEvent.data to seriali…
7cee330 
…zed color value when InputEvent.inputType is "formatBackColor" or "formatForeColor" r=smaug,m_kato,emilio

Although neither Chrome nor Safari does not set InputEvent.data when the event
is caused by `document.execCommand()` with `backColor`, `foreColor` nor
`hiliteColor`, Safari supports styling color with touchbar and in that case,
Safari sets it (*1).

Additionally, currently Safari uses `rgb()` to represents a color value and
using same rule to serializing color value for CSS OM matches Safari's behavior
and can represent any valid color values.

This patch makes given color value parsed and then serialized with same code
in style system.  If the value is `currentcolor`, `inherit`, `initial` or `reset`, sets
the value as-is for now.  Additionally, when given value is invalid, sets the value
as-is for forward compatibility.

Note that automated tests will be added into input-events-exec-command.html
by the last patch.

1. w3c/input-events#94 (comment)

Differential Revision: https://phabricator.services.mozilla.com/D19295

UltraBlame original commit: ef76c4985c1a632742814949279816207396ff38
gecko-dev-updater pushed a commit to marco-c/gecko-dev-comments-removed that referenced this issue Oct 4, 2019
@marco-c
Bug 998941 - part 1-7: Make HTMLEditor set InputEvent.data to seriali…
6b52291 
…zed color value when InputEvent.inputType is "formatBackColor" or "formatForeColor" r=smaug,m_kato,emilio

Although neither Chrome nor Safari does not set InputEvent.data when the event
is caused by `document.execCommand()` with `backColor`, `foreColor` nor
`hiliteColor`, Safari supports styling color with touchbar and in that case,
Safari sets it (*1).

Additionally, currently Safari uses `rgb()` to represents a color value and
using same rule to serializing color value for CSS OM matches Safari's behavior
and can represent any valid color values.

This patch makes given color value parsed and then serialized with same code
in style system.  If the value is `currentcolor`, `inherit`, `initial` or `reset`, sets
the value as-is for now.  Additionally, when given value is invalid, sets the value
as-is for forward compatibility.

Note that automated tests will be added into input-events-exec-command.html
by the last patch.

1. w3c/input-events#94 (comment)

Differential Revision: https://phabricator.services.mozilla.com/D19295

UltraBlame original commit: ef76c4985c1a632742814949279816207396ff38
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified-and-comments-removed that referenced this issue Oct 4, 2019
@marco-c
Bug 998941 - part 1-7: Make HTMLEditor set InputEvent.data to seriali…
3918a27 
…zed color value when InputEvent.inputType is "formatBackColor" or "formatForeColor" r=smaug,m_kato,emilio

Although neither Chrome nor Safari does not set InputEvent.data when the event
is caused by `document.execCommand()` with `backColor`, `foreColor` nor
`hiliteColor`, Safari supports styling color with touchbar and in that case,
Safari sets it (*1).

Additionally, currently Safari uses `rgb()` to represents a color value and
using same rule to serializing color value for CSS OM matches Safari's behavior
and can represent any valid color values.

This patch makes given color value parsed and then serialized with same code
in style system.  If the value is `currentcolor`, `inherit`, `initial` or `reset`, sets
the value as-is for now.  Additionally, when given value is invalid, sets the value
as-is for forward compatibility.

Note that automated tests will be added into input-events-exec-command.html
by the last patch.

1. w3c/input-events#94 (comment)

Differential Revision: https://phabricator.services.mozilla.com/D19295

UltraBlame original commit: ef76c4985c1a632742814949279816207396ff38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@rniwa @johanneswilm @whsieh @masayuki-nakano