Page MenuHomePhabricator

[Regression pre-wmf4] Make getNodeAndOffset safe with both IMEs and slugless zero-DM-length branch nodes
Closed, ResolvedPublic8 Story Points

Description

Change Iea6656882293e8e3cc78df87c31afe58ff59c7e8 fixed T110378 , which was caused by getNodeAndOffset crashing on slugless branch nodes with zero DM length. The fix was to re-rendering the nodes from inside getNodeAndOffset, thereby adding slugs to zero-length branch nodes.

However re-rendering can close IME candidate windows and prematurely commit text, so we should avoid such a call inside an apparently pure function.

Slugless branch nodes with zero DM length can happen when all the content is deleted by a browser native action (without a re-render). At first I thought perhaps we could move the re-render calls to IME-safe places in the code, such that getNodeAndOffset could never be called on such a node. Unfortunately, it turns out to be impossible because of IME restrictions:

  1. An IME can remove all the text out of the node at a time when it is unsafe to re-render.
  2. For SurfaceObserver to poll changes, getNodeAndOffset must be called at this time.

Therefore, slugless branch nodes with zero DM length cannot be avoided, and getNodeAndOffset must support them directly.

Event Timeline

dchan created this task.Oct 18 2015, 1:28 PM
dchan claimed this task.
dchan raised the priority of this task from to Normal.
dchan updated the task description. (Show Details)
dchan added subscribers: Esanders, DLynch.
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptOct 18 2015, 1:28 PM

Change 247090 had a related patch set uploaded (by Divec):
getNodeAndOffset: handle slugless nodes with zero DM length

https://gerrit.wikimedia.org/r/247090

dchan updated the task description. (Show Details)Oct 18 2015, 1:48 PM
dchan set Security to None.

Change 247090 merged by jenkins-bot:
getNodeAndOffset: handle slugless nodes with zero DM length

https://gerrit.wikimedia.org/r/247090

Jdforrester-WMF renamed this task from Make getNodeAndOffset safe with both IMEs and slugless zero-DM-length branch nodes to [Regression pre-wmf4] Make getNodeAndOffset safe with both IMEs and slugless zero-DM-length branch nodes.Oct 19 2015, 1:28 AM
Jdforrester-WMF closed this task as Resolved.
Jdforrester-WMF edited a custom field.