Bug 205913 – [GTK] Editing: Allow line breaks in lists

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKitGTK

[Milan Crha]

Moving this from a downstream report:
https://gitlab.gnome.org/GNOME/evolution/issues/747

Ordered and unorder lists can contain ("soft") line breaks, but the WebKitGTK+ editor doesn't allow it.

That is, you can have an unordered list like this:

    * line 1 of item 1
      line 2 of item 1
    * item 2

Trying with Libre Office, there can be used Shift+Enter to insert the ("soft") line break into the list item. In WebKitGTK+ the Shift+Enter behaves the same as plain Enter key press. This had been tested with WebKitGTK+ webkit2.26 branch at r252187.

Steps:
a) run MiniBrowser --editor-mode
b) right-click in the body and pick "Inspect Element"
c) switch to the console tab and execute there:
   document.execCommand("insertunorderedlist", false, "")
d) click into the body (the cursor is after the '*' of the inserted unordered list)
e) type: abc
f) press Shift+Enter

It adds a new list item:

   * abc
   * |

instead of soft line break within the current list item:

   * abc
     |

(where the '|' means the caret position).

[Milan Crha]

Created attachment 387092 [details]
proposed patch

Here is a proposed patch. It's not only for lists, but I think it's fine.

Notes: I wanted to use "prepare-ChangeLog" script, but it failed with the below error. It doesn't know I changed anything, even the 'git status' says:

> On branch webkit-2.26
> Changes not staged for commit:
>   (use "git add <file>..." to update what will be committed)
>   (use "git checkout -- <file>..." to discard changes in working directory)
>
> 	modified:   Source/WebKit/UIProcess/gtk/KeyBindingTranslator.cpp
>
> no changes added to commit (use "git add" and/or "git commit -a")

Thus there's something fishy about it, or the script works differently since the last time I used it (which is years ago). As this failed, I didn't go through step 7 of the https://webkit.org/contributing-code/ ; I just made the patch with `git diff`.

-----------------------------------------------------------------------------

The error output promised at the top:

$ Tools/Scripts/prepare-ChangeLog --style -b https://bugs.webkit.org/show_bug.cgi?id=205913
  Running check-webkit-style.
  Traceback (most recent call last):
  File "/data/develop/test-wk2/_other/webkit.master/Tools/Scripts/check-webkit-style", line 48, in <module>
    sys.exit(CheckWebKitStyle().main())
  File "/data/develop/test-wk2/_other/webkit.master/Tools/Scripts/webkitpy/style/main.py", line 153, in main
    patch = host.scm().create_patch(options.git_commit, changed_files=changed_files, git_index=options.git_index)
  File "/data/develop/test-wk2/_other/webkit.master/Tools/Scripts/webkitpy/common/checkout/scm/git.py", line 354, in create_patch
    command += [self.merge_base(git_commit)]
  File "/data/develop/test-wk2/_other/webkit.master/Tools/Scripts/webkitpy/common/checkout/scm/git.py", line 215, in merge_base
    return self.remote_merge_base()
  File "/data/develop/test-wk2/_other/webkit.master/Tools/Scripts/webkitpy/common/checkout/scm/git.py", line 528, in remote_merge_base
    return self._run_git(['merge-base', self.remote_branch_ref(), 'HEAD']).strip()
  File "/data/develop/test-wk2/_other/webkit.master/Tools/Scripts/webkitpy/common/checkout/scm/git.py", line 106, in _run_git
    return self.run(full_command_args, **full_kwargs)
  File "/data/develop/test-wk2/_other/webkit.master/Tools/Scripts/webkitpy/common/checkout/scm/scm.py", line 78, in run
    decode_output=decode_output)
  File "/data/develop/test-wk2/_other/webkit.master/Tools/Scripts/webkitpy/common/system/executive.py", line 412, in run_command
    (error_handler or self.default_error_handler)(script_error)
  File "/data/develop/test-wk2/_other/webkit.master/Tools/Scripts/webkitpy/common/system/abstractexecutive.py", line 97, in default_error_handler
    raise error
webkitpy.common.system.executive.ScriptError: Failed to run "['git', 'merge-base', 'refs/remotes/origin/master', 'HEAD']" exit_code: 1 cwd: /data/develop/test-wk2/_other/webkit.master
  Running status to find changed, added, or removed files.
  No changes found.


$ Tools/Scripts/prepare-ChangeLog --nostyle -b https://bugs.webkit.org/show_bug.cgi?id=205913
  Running status to find changed, added, or removed files.
  No changes found.

I also tried "git add Source/WebKit/UIProcess/gtk/KeyBindingTranslator.cpp", but the prepare-ChangeLog didn't find the changes anyway.

[Carlos Garcia Campos]

Comment on attachment 387092 [details]
proposed patch

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

r- because the changelog is not correct, cq won't allow this in any case.

> Source/WebKit/ChangeLog:9
> +	Treat Shift+Enter as "InsertLineBreak", instead of "InsertParagraph".

I guess you mean instead of "InsertNewLine"

> Source/WebKit/ChangeLog:10
> +

This ChangeLog is not correctly formatted... we need to figure out why the script doesn't work for you.

> Source/WebKit/UIProcess/gtk/KeyBindingTranslator.cpp:217
>      // Special-case enter keys for we want them to work regardless of modifier.

This comment is no longer accurate. Could we just handle enter keys as all other custom key bindings and add them to the table above and just reove this special case?

[Milan Crha]

(In reply to Carlos Garcia Campos from comment #2)
> I guess you mean instead of "InsertNewLine"

Oops, that's right. I wrote it off-head, my fault.

> This ChangeLog is not correctly formatted... we need to figure out why the
> script doesn't work for you.

Right. Any idea how to debug it? I tried to switch from the webkit-2.26 git branch to the master branch and it made the style checked happy, but the prepare-ChangeLog still claims that it didn't find any change.

> Could we just handle enter keys as all other custom key bindings and add
> them to the table above and just reove this special case?

Okay. It uncovered a bug in that code. When I've a NumLock on, then the 'state' variable says 0x10, which doesn't match any of the combined state and keycode values. Thus I fixed that too, by considering only Ctrl+Alt+Shift keys from the 'state'.

I reformatted whole "table", because the "InsertLineBreak" is longer than "InsertBacktab", thus it would need longer lines anyway.

[Milan Crha]

Created attachment 387101 [details]
proposed patch ][

[Carlos Garcia Campos]

Comment on attachment 387101 [details]
proposed patch ][

Thanks!

[WebKit Commit Bot]

Comment on attachment 387101 [details]
proposed patch ][

Clearing flags on attachment: 387101

Committed r254199: <https://trac.webkit.org/changeset/254199>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.