[Webkit-unassigned] [Bug 161546] Need to updateEditorState if an element change edit-ability without changing selection

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=161546

Ryosuke Niwa <rniwa at webkit.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #287811|review?                     |review+
              Flags|                            |

--- Comment #4 from Ryosuke Niwa <rniwa at webkit.org> ---
Comment on attachment 287811
  --> https://bugs.webkit.org/attachment.cgi?id=287811
Patch

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

r=me assuming you fix builds & tests and stuff.

> Source/WebCore/editing/FrameSelection.cpp:2386
> +        client->updateEditorStateAfterLayoutIfNeeded();

Instead of saying IfNeeded, can we explicitly say IfEditabilityChanged to make semantics clear?

> Source/WebCore/page/EditorClient.h:187
> +
> +    virtual void updateEditorStateAfterLayoutIfNeeded() = 0;

We should add this right at where didChangeSelectionAndUpdateLayout is declared.

> Source/WebKit2/WebProcess/WebCoreSupport/WebEditorClient.cpp:557
> +void WebEditorClient::updateEditorStateAfterLayoutIfNeeded()
> +{
> +    m_page->updateEditorStateAfterLayoutIfNeeded();
> +}
> +

Ditto about defining this next to didChangeSelectionAndUpdateLayout.

> Source/WebKit2/WebProcess/WebCoreSupport/WebEditorClient.h:172
> +    void updateEditorStateAfterLayoutIfNeeded() final;
> +

Ditto about the location.

> Source/WebKit2/WebProcess/WebPage/WebPage.cpp:910
> +    EditorStateIsContentEditable editorStateIsContentEditable = frame.selection().selection().isContentEditable() ? EditorStateIsContentEditable::Yes : EditorStateIsContentEditable::No;

This doesn't detect the case when the value changes between plaintextonly and true.
It's probably not a big deal but you can check that condition by checking both isContentEditable and isContentRichlyEditable.
You might wanna add FIXME for now.

> Source/WebKit2/WebProcess/WebPage/WebPage.h:1479
> +    EditorStateIsContentEditable m_lastEditorStateWasContentEditable { EditorStateIsContentEditable::Unset };

Should we just make this mutable instead of removing const from editorState?

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.webkit.org/pipermail/webkit-unassigned/attachments/20160902/065010fc/attachment.html>