Closed
Bug 515685
Opened 15 years ago
Closed 15 years ago
Calculate modified-text mutation only when needed
Categories
(Core :: Disability Access APIs, defect)
Core
Disability Access APIs
Tracking
()
RESOLVED
FIXED
People
(Reporter: davidb, Assigned: davidb)
References
Details
(Keywords: access, perf)
Attachments
(1 file, 2 obsolete files)
|
(deleted),
patch
|
surkov
:
review+
|
Details | Diff | Splinter Review |
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.
Attachment #399767 -
Flags: review?(surkov.alexander)
Attachment #399767 -
Flags: review?(marco.zehe)
|
Assignee | |
Updated•15 years ago
|
Attachment #399767 -
Attachment description: patch 1 → wip
Attachment #399767 -
Flags: review?(surkov.alexander)
Attachment #399767 -
Flags: review?(marco.zehe)
|
|
Assignee | |
Comment 1•15 years ago
|
||
Builds on windows, passes mochitests cleanly. Needs to be tested manually since the changes hit our windows wrap layer.
Assignee: nobody → bolterbugz
Attachment #399767 -
Attachment is obsolete: true
Status: NEW → ASSIGNED
Attachment #399802 -
Flags: review?(marco.zehe)
|
|
Assignee | |
Updated•15 years ago
|
Attachment #399802 -
Flags: review?(surkov.alexander)
|
||
Comment 2•15 years ago
|
||
This should broke delayed text events, especially on text removing.
|
|
Assignee | |
Comment 3•15 years ago
|
||
(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.
|
|
Assignee | |
Comment 4•15 years ago
|
||
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.
Attachment #399802 -
Attachment is obsolete: true
Attachment #400016 -
Flags: review?(surkov.alexander)
Attachment #399802 -
Flags: review?(surkov.alexander)
Attachment #399802 -
Flags: review?(marco.zehe)
|
|
||
Updated•15 years ago
|
Attachment #400016 -
Attachment is patch: true
Attachment #400016 -
Attachment mime type: application/octet-stream → text/plain
|
|
||
Comment 5•15 years ago
|
||
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).
|
|
Assignee | |
Comment 6•15 years ago
|
||
(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•15 years ago
|
||
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.
Attachment #400016 -
Flags: review?(surkov.alexander) → review+
|
|
Assignee | |
Comment 8•15 years ago
|
||
Filed follow up bug 516123
pushed http://hg.mozilla.org/mozilla-central/rev/ddcbcbabb4ce
Status: ASSIGNED → RESOLVED
Closed: 15 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•