Bug 118346 – [EFL] Support some more editing keyboard events

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKit EFL

[KyungTae Kim]

Thanks to bug 89268, editing keyboard events is supported for EFL WK2, 
but some more common keyboard events are needed - select all, undo, redo, copy, paste and cut.
The copy, paste and cut are not work well now due to the lack of implementation on a clipboard, but it'll be implemented soon.

[KyungTae Kim]

Created attachment 205986 [details]
Patch

[Chris Dumez]

Comment on attachment 205986 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=205986&action=review

> Source/WebCore/ChangeLog:10
> +        No new tests, no behavior change.

How can there be no behavior change?

[Gyuyoung Kim]

Comment on attachment 205986 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=205986&action=review

> Source/WebCore/ChangeLog:3
> +        [EFL][WK2] Support some more editing keyboard events

This isn't only for WK2. Remove [WK2] prefix.

> Source/WebCore/ChangeLog:10
> +        No new tests, no behavior change.

I wonder if there are unskip tests in editing layout test.

[KyungTae Kim]

(In reply to comment #2)
> (From update of attachment 205986 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=205986&action=review
> 
> > Source/WebCore/ChangeLog:10
> > +        No new tests, no behavior change.
> 
> How can there be no behavior change?

What I meant was no new tests needed because no behavior for LayoutTests changed.
I'll remove that description because it can make confusion.


> I wonder if there are unskip tests in editing layout test.
OK. I'm checking now if there are newly passed tests.

[Chris Dumez]

(In reply to comment #4)
> (In reply to comment #2)
> > (From update of attachment 205986 [details] [details])
> > View in context: https://bugs.webkit.org/attachment.cgi?id=205986&action=review
> > 
> > > Source/WebCore/ChangeLog:10
> > > +        No new tests, no behavior change.
> > 
> > How can there be no behavior change?
> 
> What I meant was no new tests needed because no behavior for LayoutTests changed.
> I'll remove that description because it can make confusion.

I don't understand why it wouldn't affect layout tests. What is the impact then? Why do we need this change?

[KyungTae Kim]

(In reply to comment #5)
> (In reply to comment #4)
> > (In reply to comment #2)
> > > (From update of attachment 205986 [details] [details] [details])
> > > View in context: https://bugs.webkit.org/attachment.cgi?id=205986&action=review
> > > 
> > > > Source/WebCore/ChangeLog:10
> > > > +        No new tests, no behavior change.
> > > 
> > > How can there be no behavior change?
> > 
> > What I meant was no new tests needed because no behavior for LayoutTests changed.
> > I'll remove that description because it can make confusion.
> 
> I don't understand why it wouldn't affect layout tests. What is the impact then? Why do we need this change?

With this change, you can use more shortcut keys on editable contents.
You can select all the texts with "Ctrl+A", undo the change with "Ctrl+Z", redo the change with "Ctrl+Y".
They're common functions for the editable contents but you couldn't use the functionality only in the EFL port because there were no shortcut.

[Chris Dumez]

(In reply to comment #6)
> (In reply to comment #5)
> > (In reply to comment #4)
> > > (In reply to comment #2)
> > > > (From update of attachment 205986 [details] [details] [details] [details])
> > > > View in context: https://bugs.webkit.org/attachment.cgi?id=205986&action=review
> > > > 
> > > > > Source/WebCore/ChangeLog:10
> > > > > +        No new tests, no behavior change.
> > > > 
> > > > How can there be no behavior change?
> > > 
> > > What I meant was no new tests needed because no behavior for LayoutTests changed.
> > > I'll remove that description because it can make confusion.
> > 
> > I don't understand why it wouldn't affect layout tests. What is the impact then? Why do we need this change?
> 
> With this change, you can use more shortcut keys on editable contents.
> You can select all the texts with "Ctrl+A", undo the change with "Ctrl+Z", redo the change with "Ctrl+Y".
> They're common functions for the editable contents but you couldn't use the functionality only in the EFL port because there were no shortcut.

so it looks like it would affect layout tests. Please unskip / rebase those tests in the same patch.

[KyungTae Kim]

Created attachment 206042 [details]
Patch

[KyungTae Kim]

There was 3 new passes when I executed whole tests,
and 65 new passes when I executed only the 'editing' tests,
but it was same without this patch.
So, I think rebase is needed, but it's independent of this change.

[1] 3 new passes when executing whole tests
  editing/selection/doubleclick-beside-cr-span.html
  editing/selection/doubleclick-whitespace.html
  editing/style/5065910.html
[2] 65 new passes when executing only 1592 tests in 'editing'
  editing/execCommand/paste-1.html
  editing/inserting/before-after-input-element.html
  editing/pasteboard/3976872.html
  editing/pasteboard/4076267-2.html
  editing/pasteboard/4076267-3.html
  editing/pasteboard/4076267.html
  editing/pasteboard/4242293.html
  editing/pasteboard/4641033.html
  editing/pasteboard/4944770-1.html
  editing/pasteboard/4944770-2.html
  editing/pasteboard/4947130.html
  editing/pasteboard/4989774.html
  editing/pasteboard/5006779.html
  editing/pasteboard/5028447.html
  editing/pasteboard/5071074.html
  editing/pasteboard/5075944.html
  editing/pasteboard/5478250.html
  editing/pasteboard/5601583-1.html
  editing/pasteboard/cut-text-001.html
  editing/pasteboard/drag-drop-dead-frame.html
  editing/pasteboard/drag-drop-modifies-page.html
  editing/pasteboard/drag-image-to-contenteditable-in-iframe.html
  editing/pasteboard/drag-selected-image-to-contenteditable.html
  editing/pasteboard/drop-text-without-selection.html
  editing/pasteboard/emacs-ctrl-a-k-y.html
  editing/pasteboard/emacs-ctrl-k-y-001.html
  editing/pasteboard/input-field-1.html
  editing/pasteboard/paste-2.html
  editing/pasteboard/paste-4038267-fix.html
  editing/pasteboard/paste-line-endings-001.html
  editing/pasteboard/paste-line-endings-002.html
  editing/pasteboard/paste-line-endings-003.html
  editing/pasteboard/paste-line-endings-004.html
  editing/pasteboard/paste-line-endings-005.html
  editing/pasteboard/paste-line-endings-006.html
  editing/pasteboard/paste-line-endings-007.html
  editing/pasteboard/paste-line-endings-008.html
  editing/pasteboard/paste-line-endings-009.html
  editing/pasteboard/paste-line-endings-010.html
  editing/pasteboard/paste-match-style-001.html
  editing/pasteboard/paste-match-style-002.html
  editing/pasteboard/paste-text-004.html
  editing/pasteboard/paste-text-008.html
  editing/pasteboard/paste-text-009.html
  editing/pasteboard/paste-text-013.html
  editing/pasteboard/paste-text-014.html
  editing/pasteboard/paste-text-016.html
  editing/pasteboard/paste-text-019.html
  editing/pasteboard/paste-text-at-tabspan-003.html
  editing/pasteboard/paste-xml.xhtml
  editing/pasteboard/pasting-tabs.html
  editing/pasteboard/smart-drag-drop.html
  editing/pasteboard/smart-paste-007.html
  editing/pasteboard/smart-paste-008.html
  editing/pasteboard/styled-element-markup.html
  editing/pasteboard/subframe-dragndrop-1.html
  editing/pasteboard/undoable-fragment-removes.html
  editing/selection/contains-boundaries.html
  editing/selection/doubleclick-beside-cr-span.html
  editing/selection/doubleclick-whitespace.html
  editing/selection/mixed-editability-10.html
  editing/selection/paragraph-granularity.html
  editing/selection/replaced-boundaries-3.html
  editing/style/5065910.html
  editing/style/font-family-with-space.html

[Gyuyoung Kim]

Comment on attachment 206042 [details]
Patch

Looks ok based on layout test result.

[Gyuyoung Kim]

(In reply to comment #9)
> There was 3 new passes when I executed whole tests,
> and 65 new passes when I executed only the 'editing' tests,
> but it was same without this patch.
> So, I think rebase is needed, but it's independent of this change.

Can you do gardening them ?

[KyungTae Kim]

(In reply to comment #11)
> (In reply to comment #9)
> > There was 3 new passes when I executed whole tests,
> > and 65 new passes when I executed only the 'editing' tests,
> > but it was same without this patch.
> > So, I think rebase is needed, but it's independent of this change.
> 
> Can you do gardening them ?
OK. I'll do it on another bug.

[WebKit Commit Bot]

Comment on attachment 206042 [details]
Patch

Clearing flags on attachment: 206042

Committed r152389: <http://trac.webkit.org/changeset/152389>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.