Created attachment 374825 [details] [diff] [review] Patch String::append doesn't calculate the start correctly for dependent strings if k16 or wider. (see attachment) Bug was found in Flash integration and is hard to replicate from pure AS3. Michael, if you agree with the suggested fix, can you suggest an acceptance test?
Attachment #374825 - Flags: review?(daumling)
(In reply to comment #0) > > Bug was found in Flash integration and is hard to replicate from pure AS3. But it is probably the kind of thing that should be straightforward to test from the selftest harness, and the acceptance test could be written on that level perhaps.
Comment on attachment 374825 [details] [diff] [review] Patch It should have been / thisWidth instead of * thisWidth to determine the character offset position. Anyway, the fix only enables the creation of a substring that spans the master plus additional characters. I was unable to find a test case that favored the bug, because the bug always caused the code to fall back to creating a dynamic copy. I believe that the bug is elsewhere, but for this, I would need more data. Is there a SWF that repros the bug?
Attachment #374825 - Flags: review?(daumling) → review-
Created attachment 375047 [details] SWF that repros the bug Enclosed is a SWF that demonstrates the issue. To repro, run the attached SWF. Put your breakpoint at line 103 in TextElementGlue.cpp, in the method DoReplaceText. On the 10th hit, beginIndex is 318 and you can see the bug happen. (Note, this only happens reliably on Windows; due to device-font issues the rendering is different on different platforms.)
Created attachment 376506 [details] [diff] [review] Start offset for dependent strings was incorrect
Created attachment 376507 [details] Testcase The problem was not the fix, but the testcase. This testcase reproduces the scenario inside the SWF, and it shows the bug clearly.
Attachment #376507 - Attachment mime type: application/octet-stream → text/plain
Comment on attachment 376506 [details] [diff] [review] Start offset for dependent strings was incorrect nice, let's push the acceptance test along with this
Attachment #376506 - Flags: review?(stejohns) → review+
pushed to redux as changeset: 1860:f68ec0f2da1e
Status: NEW → RESOLVED
Last Resolved: 10 years ago
Resolution: --- → FIXED
testcase has been running in all builders, verifying fix
Status: RESOLVED → VERIFIED
Comment on attachment 375047 [details] SWF that repros the bug Because the bug is fixed, the review is obsolete.
You need to log in before you can comment on or make changes to this bug.