Attachment #223528 for bug #128421

View Details Raw Unified Return to Bug 128421

| Differences between



2014-02-07  Ryosuke Niwa  <rniwa@webkit.org>

        FrameSelection's destructor shouldn't notify accessibility
        https://bugs.webkit.org/show_bug.cgi?id=128421

        Reviewed by NOBODY (OOPS!).

        Extracted setSelectionWithoutUpdatingAppearance out of setSelection and called it in prepareForDestruction
        instead of setting DoNotUpdateAppearance option. This new function doesn't reveal selection or notify
        accessibility code in addition to not updating appearance.

        Note that all implementations of notifyAccessibilityForSelectionChange in FrameSelectionAtk.cpp and
        FrameSelectionMac.mm exit early when the selection type is not caret or either start or end is null,
        which is already the case inside FrameSelection's destructor. In addition, revealSelection is never called
        when selection change was not triggered by user so there should be no behavioral change from this patch.

        * editing/FrameSelection.cpp:
        (WebCore::FrameSelection::setSelectionWithoutUpdatingAppearance):
        (WebCore::FrameSelection::setSelection):
        (WebCore::FrameSelection::prepareForDestruction):
        * editing/FrameSelection.h:
        (WebCore::FrameSelection::notifyAccessibilityForSelectionChange): Added the trivial implementation in the case
        accessibility is disabled to tidy up call sites.

2014-02-07  Roger Fong  <roger_fong@apple.com>

        CGContextGetUserSpaceToDeviceSpaceTransform returns the wrong value on Windows.



    setSelection(newSelection, UserTriggered | DoNotRevealSelection | CloseTyping | ClearTypingStyle, AlignCursorOnScrollIfNeeded, granularity);
}

void FrameSelection::setSelectionWithoutUpdatingAppearance(const VisibleSelection& newSelection, SetSelectionOptions options, CursorAlignOnScroll align, TextGranularity granularity, EUserTriggered userTriggered)
{
    bool closeTyping = options & CloseTyping;
    bool shouldClearTypingStyle = options & ClearTypingStyle;
    EUserTriggered userTriggered = selectionOptionsToUserTriggered(options);

    VisibleSelection s = newSelection;
    if (shouldAlwaysUseDirectionalSelection(m_frame))

    if (!s.isNone() && !(options & DoNotSetFocus))
        setFocusedElementIfNeeded();

    if (!(options & DoNotUpdateAppearance)) {
#if ENABLE(TEXT_CARET)
        m_frame->document()->updateLayoutIgnorePendingStylesheets();
#else
        m_frame->document()->updateStyleIfNeeded();
#endif
        updateAppearance();
    }

    // Always clear the x position used for vertical arrow navigation.
    // It will be restored by the vertical arrow navigation code if necessary.
    m_xPosForVerticalArrowNavigation = NoXPosForVerticalArrowNavigation();
    selectFrameElementInParentIfFullySelected();
    updateSelectionCachesIfSelectionIsInsideTextFormControl(userTriggered);
    m_frame->editor().respondToChangedSelection(oldSelection, options);
    m_frame->document()->enqueueDocumentEvent(Event::create(eventNames().selectionchangeEvent, false, false));
}

void FrameSelection::setSelection(const VisibleSelection& selection, SetSelectionOptions options, CursorAlignOnScroll align, TextGranularity granularity)
{
    EUserTriggered userTriggered = selectionOptionsToUserTriggered(options);

    setSelectionWithoutUpdatingAppearance(selection, options, align, granularity, userTriggered);

#if ENABLE(TEXT_CARET)
    m_frame->document()->updateLayoutIgnorePendingStylesheets();
#else
    m_frame->document()->updateStyleIfNeeded();
#endif

    updateAppearance();

    if (userTriggered == UserTriggered && !(options & DoNotRevealSelection)) {
        ScrollAlignment alignment;



        revealSelection(alignment, RevealExtent);
    }

    notifyAccessibilityForSelectionChange();
#endif
    m_frame->document()->enqueueDocumentEvent(Event::create(eventNames().selectionchangeEvent, false, false));
}

static bool removingNodeRemovesPosition(Node* node, const Position& position)

    if (view)
        view->clearSelection();

    setSelectionWithoutUpdatingAppearance(VisibleSelection(), CloseTyping | ClearTypingStyle,
        AlignCursorOnScrollIfNeeded, CharacterGranularity, NotUserTriggered);
    m_previousCaretNode.clear();
}


Lines 124-131 public: a/Source/WebCore/editing/FrameSelection.h_sec1
- a/Source/WebCore/editing/FrameSelection.h -2 / +5 lines
124	SpellCorrectionTriggered = 1 << 3,	124	SpellCorrectionTriggered = 1 << 3,
125	DoNotSetFocus = 1 << 4,	125	DoNotSetFocus = 1 << 4,
126	DictationTriggered = 1 << 5,	126	DictationTriggered = 1 << 5,
127	DoNotUpdateAppearance = 1 << 6,	127	DoNotRevealSelection = 1 << 6,
128	DoNotRevealSelection = 1 << 7,
129	};	128	};
130	typedef unsigned SetSelectionOptions; // Union of values in SetSelectionOption and EUserTriggered	129	typedef unsigned SetSelectionOptions; // Union of values in SetSelectionOption and EUserTriggered
131	static inline EUserTriggered selectionOptionsToUserTriggered(SetSelectionOptions options)	130	static inline EUserTriggered selectionOptionsToUserTriggered(SetSelectionOptions options)
Lines 280-285 public: a/Source/WebCore/editing/FrameSelection.h_sec2
280	private:	279	private:
281	enum EPositionType { START, END, BASE, EXTENT };	280	enum EPositionType { START, END, BASE, EXTENT };
282		281
		282	void setSelectionWithoutUpdatingAppearance(const VisibleSelection&, SetSelectionOptions, CursorAlignOnScroll, TextGranularity, EUserTriggered);
		283
283	void respondToNodeModification(Node*, bool baseRemoved, bool extentRemoved, bool startRemoved, bool endRemoved);	284	void respondToNodeModification(Node*, bool baseRemoved, bool extentRemoved, bool startRemoved, bool endRemoved);
284	TextDirection directionOfEnclosingBlock();	285	TextDirection directionOfEnclosingBlock();
285	TextDirection directionOfSelection();	286	TextDirection directionOfSelection();
Lines 302-307 private: a/Source/WebCore/editing/FrameSelection.h_sec3
302		303
303	#if HAVE(ACCESSIBILITY)	304	#if HAVE(ACCESSIBILITY)
304	void notifyAccessibilityForSelectionChange();	305	void notifyAccessibilityForSelectionChange();
		306	#else
		307	void notifyAccessibilityForSelectionChange() { }
305	#endif	308	#endif
306		309
307	void updateSelectionCachesIfSelectionIsInsideTextFormControl(EUserTriggered);	310	void updateSelectionCachesIfSelectionIsInsideTextFormControl(EUserTriggered);

Return to Bug 128421

Lines 250-260 void FrameSelection::setSelectionByMouseIfDifferent(const VisibleSelection& pass a/Source/WebCore/editing/FrameSelection.cpp_sec1
- a/Source/WebCore/editing/FrameSelection.cpp -15 / +21 lines
250	setSelection(newSelection, UserTriggered \| DoNotRevealSelection \| CloseTyping \| ClearTypingStyle, AlignCursorOnScrollIfNeeded, granularity);	250	setSelection(newSelection, UserTriggered \| DoNotRevealSelection \| CloseTyping \| ClearTypingStyle, AlignCursorOnScrollIfNeeded, granularity);
251	}	251	}
252		252
253	void FrameSelection::setSelection(const VisibleSelection& newSelection, SetSelectionOptions options, CursorAlignOnScroll align, TextGranularity granularity)	253	void FrameSelection::setSelectionWithoutUpdatingAppearance(const VisibleSelection& newSelection, SetSelectionOptions options, CursorAlignOnScroll align, TextGranularity granularity, EUserTriggered userTriggered)
254	{	254	{
255	bool closeTyping = options & CloseTyping;	255	bool closeTyping = options & CloseTyping;
256	bool shouldClearTypingStyle = options & ClearTypingStyle;	256	bool shouldClearTypingStyle = options & ClearTypingStyle;
257	EUserTriggered userTriggered = selectionOptionsToUserTriggered(options);
258		257
259	VisibleSelection s = newSelection;	258	VisibleSelection s = newSelection;
260	if (shouldAlwaysUseDirectionalSelection(m_frame))	259	if (shouldAlwaysUseDirectionalSelection(m_frame))
Lines 307-327 void FrameSelection::setSelection(const VisibleSelection& newSelection, SetSelec a/Source/WebCore/editing/FrameSelection.cpp_sec2
307	if (!s.isNone() && !(options & DoNotSetFocus))	306	if (!s.isNone() && !(options & DoNotSetFocus))
308	setFocusedElementIfNeeded();	307	setFocusedElementIfNeeded();
309		308
310	if (!(options & DoNotUpdateAppearance)) {
311	#if ENABLE(TEXT_CARET)
312	m_frame->document()->updateLayoutIgnorePendingStylesheets();
313	#else
314	m_frame->document()->updateStyleIfNeeded();
315	#endif
316	updateAppearance();
317	}
318
319	// Always clear the x position used for vertical arrow navigation.	309	// Always clear the x position used for vertical arrow navigation.
320	// It will be restored by the vertical arrow navigation code if necessary.	310	// It will be restored by the vertical arrow navigation code if necessary.
321	m_xPosForVerticalArrowNavigation = NoXPosForVerticalArrowNavigation();	311	m_xPosForVerticalArrowNavigation = NoXPosForVerticalArrowNavigation();
322	selectFrameElementInParentIfFullySelected();	312	selectFrameElementInParentIfFullySelected();
323	updateSelectionCachesIfSelectionIsInsideTextFormControl(userTriggered);	313	updateSelectionCachesIfSelectionIsInsideTextFormControl(userTriggered);
324	m_frame->editor().respondToChangedSelection(oldSelection, options);	314	m_frame->editor().respondToChangedSelection(oldSelection, options);
		315	m_frame->document()->enqueueDocumentEvent(Event::create(eventNames().selectionchangeEvent, false, false));
		316	}
		317
		318	void FrameSelection::setSelection(const VisibleSelection& selection, SetSelectionOptions options, CursorAlignOnScroll align, TextGranularity granularity)
		319	{
		320	EUserTriggered userTriggered = selectionOptionsToUserTriggered(options);
		321
		322	setSelectionWithoutUpdatingAppearance(selection, options, align, granularity, userTriggered);
		323
		324	#if ENABLE(TEXT_CARET)
		325	m_frame->document()->updateLayoutIgnorePendingStylesheets();
		326	#else
		327	m_frame->document()->updateStyleIfNeeded();
		328	#endif
		329
		330	updateAppearance();
		331
325	if (userTriggered == UserTriggered && !(options & DoNotRevealSelection)) {	332	if (userTriggered == UserTriggered && !(options & DoNotRevealSelection)) {
326	ScrollAlignment alignment;	333	ScrollAlignment alignment;
327		334
Lines 332-341 void FrameSelection::setSelection(const VisibleSelection& newSelection, SetSelec a/Source/WebCore/editing/FrameSelection.cpp_sec3
332		339
333	revealSelection(alignment, RevealExtent);	340	revealSelection(alignment, RevealExtent);
334	}	341	}
335	#if HAVE(ACCESSIBILITY)	342
336	notifyAccessibilityForSelectionChange();	343	notifyAccessibilityForSelectionChange();
337	#endif
338	m_frame->document()->enqueueDocumentEvent(Event::create(eventNames().selectionchangeEvent, false, false));
339	}	344	}
340		345
341	static bool removingNodeRemovesPosition(Node* node, const Position& position)	346	static bool removingNodeRemovesPosition(Node* node, const Position& position)
Lines 1237-1243 void FrameSelection::prepareForDestruction() a/Source/WebCore/editing/FrameSelection.cpp_sec4
1237	if (view)	1242	if (view)
1238	view->clearSelection();	1243	view->clearSelection();
1239		1244
1240	setSelection(VisibleSelection(), CloseTyping \| ClearTypingStyle \| DoNotUpdateAppearance);	1245	setSelectionWithoutUpdatingAppearance(VisibleSelection(), CloseTyping \| ClearTypingStyle,
		1246	AlignCursorOnScrollIfNeeded, CharacterGranularity, NotUserTriggered);
1241	m_previousCaretNode.clear();	1247	m_previousCaretNode.clear();
1242	}	1248	}
1243		1249

Lines 1-3 a/Source/WebCore/ChangeLog_sec1
- a/Source/WebCore/ChangeLog +24 lines
		1	2014-02-07 Ryosuke Niwa <rniwa@webkit.org>
		2
		3	FrameSelection's destructor shouldn't notify accessibility
		4	https://bugs.webkit.org/show_bug.cgi?id=128421
		5
		6	Reviewed by NOBODY (OOPS!).
		7
		8	Extracted setSelectionWithoutUpdatingAppearance out of setSelection and called it in prepareForDestruction
		9	instead of setting DoNotUpdateAppearance option. This new function doesn't reveal selection or notify
		10	accessibility code in addition to not updating appearance.
		11
		12	Note that all implementations of notifyAccessibilityForSelectionChange in FrameSelectionAtk.cpp and
		13	FrameSelectionMac.mm exit early when the selection type is not caret or either start or end is null,
		14	which is already the case inside FrameSelection's destructor. In addition, revealSelection is never called
		15	when selection change was not triggered by user so there should be no behavioral change from this patch.
		16
		17	* editing/FrameSelection.cpp:
		18	(WebCore::FrameSelection::setSelectionWithoutUpdatingAppearance):
		19	(WebCore::FrameSelection::setSelection):
		20	(WebCore::FrameSelection::prepareForDestruction):
		21	* editing/FrameSelection.h:
		22	(WebCore::FrameSelection::notifyAccessibilityForSelectionChange): Added the trivial implementation in the case
		23	accessibility is disabled to tidy up call sites.
		24
1	2014-02-07 Roger Fong <roger_fong@apple.com>	25	2014-02-07 Roger Fong <roger_fong@apple.com>
2		26
3	CGContextGetUserSpaceToDeviceSpaceTransform returns the wrong value on Windows.	27	CGContextGetUserSpaceToDeviceSpaceTransform returns the wrong value on Windows.