515685 - Calculate modified-text mutation only when needed

Status: RESOLVED FIXED

(Core :: Disability Access APIs, defect)

Description

[David Bolter [:davidb] (NeedInfo me for attention)]

Attachment: wip

Currently we calculate the modified (and rendered) text for every text mutation. There are cases (see blocked bugs) where we do this an awful lot. This computation should be on-demand. Patch attached.

Comment 1

[David Bolter [:davidb] (NeedInfo me for attention)]

Attachment: patch 1

Builds on windows, passes mochitests cleanly. Needs to be tested manually since the changes hit our windows wrap layer.

Comment 2

[alexander :surkov (:asurkov)]

This should broke delayed text events, especially on text removing.

Comment 3

[David Bolter [:davidb] (NeedInfo me for attention)]

(In reply to comment #2)
> This should broke delayed text events, especially on text removing.

Good point. This is presumably because the text will have changed in the meantime... darn I didn't think of that. Nice catch.

Comment 4

[David Bolter [:davidb] (NeedInfo me for attention)]

Attachment: patch 2 (ifdefs)

This is the simplest fix, and probably the best IMO. As much as I dislike ifdefs.

Not sure how to avoid it simply.

I thought about using a virtual initialization method and calling it from the constructor, but calling virtual methods during object construction doesn't do what we want in C++ because of the order of base object, then derived object construction.

Overriding the constructor itself would require the a virtual constructor which of course doesn't exist.

Comment 5

[alexander :surkov (:asurkov)]

If this resolves a problem of bug 510688 then I don't mind if the approach is temporary (because Gecko's based AT softwares expect the same behaviour on all platforms).

Comment 6

[David Bolter [:davidb] (NeedInfo me for attention)]

(In reply to comment #5)
> If this resolves a problem of bug 510688 then I don't mind if the approach is
> temporary (because Gecko's based AT softwares expect the same behaviour on all
> platforms).

Fair enough but ATK doesn't have API that asks for the modified text from the event, just that the text changed. Gecko based AT might find this enough too; if they ask for it we'll add it back :)

This patch mitigates bug 510688 but doesn't fix it (510688 has a few dependencies). Note Ginn asked for this bug fix a while back on some other bug.

Comment 7

[alexander :surkov (:asurkov)]

Comment on attachment 400016 [details] [diff] [review]
patch 2 (ifdefs)

sounds ok, r=me

David said we get things quicker at 18% percents. Therefore it's worth to take this one. But we need to file following up bug since it's temporary approach I believe.

Comment 8

[David Bolter [:davidb] (NeedInfo me for attention)]

Filed follow up bug 516123

pushed http://hg.mozilla.org/mozilla-central/rev/ddcbcbabb4ce