Bug 123973 - nextBoundary and previousBoundary are very slow when there is a password field
Summary: nextBoundary and previousBoundary are very slow when there is a password field
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: HTML Editing (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Ryosuke Niwa
URL:
Keywords: BlinkMergeCandidate
: 79815 (view as bug list)
Depends on:
Blocks:
 
Reported: 2013-11-07 01:03 PST by Ryosuke Niwa
Modified: 2013-11-22 18:01 PST (History)
8 users (show)

See Also:


Attachments
Fixes the bug (6.43 KB, patch)
2013-11-07 01:05 PST, Ryosuke Niwa
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Ryosuke Niwa 2013-11-07 01:03:09 PST
Merge https://chromium.googlesource.com/chromium/blink/+/57366eec5e3edea54062d4e74c0e047f8681dbad

When iterating through DOM nodes nextBoundary and previousBoundary
convert the contents of nodes using text security to a sequence of 'x'
characters. The SimplifiedBackwardsTextIterator and TextIterator may
iterate past node boundaries. Before this patch, the transformation was
done looking at the starting node rather than the current node. In some
situations, this replaced all boundaries with 'x' and caused the
iterator to continue iterating and transforming until the extent of the
document.
Comment 1 Ryosuke Niwa 2013-11-07 01:05:04 PST
Created attachment 216273 [details]
Fixes the bug
Comment 2 Martin Robinson 2013-11-07 08:03:22 PST
I had considered posting this patch a few weeks ago, but couldn't figure out how to reproduce the bug in WebKit. I think the fix is valid regardless.
Comment 3 Alexey Proskuryakov 2013-11-07 09:09:11 PST
Does this address bug 79815 by any chance?
Comment 4 Martin Robinson 2013-11-07 10:46:20 PST
(In reply to comment #3)
> Does this address bug 79815 by any chance?

That is the test case I was using to test this patch, but I found that even without the patch, the test case shows no sluggish behavior. Can one of you verify that the test case still shows the issue on Mac?
Comment 5 Alexey Proskuryakov 2013-11-07 10:58:38 PST
There was an observable delay when deleting the last character in password field, using Safari 7 and its included WebKit. I didn't try a nightly.
Comment 6 Martin Robinson 2013-11-07 11:37:26 PST
(In reply to comment #5)
> There was an observable delay when deleting the last character in password field, using Safari 7 and its included WebKit. I didn't try a nightly.

If this patch fixes that, I say go for it. There may be something peculiar about the GTK+ port that means it doesn't affect us.
Comment 7 WebKit Commit Bot 2013-11-21 05:44:15 PST
Comment on attachment 216273 [details]
Fixes the bug

Clearing flags on attachment: 216273

Committed r159619: <http://trac.webkit.org/changeset/159619>
Comment 8 WebKit Commit Bot 2013-11-21 05:44:18 PST
All reviewed patches have been landed.  Closing bug.
Comment 9 Alexey Proskuryakov 2013-11-21 10:08:47 PST
The editing/deleting/password-delete-performance.html test is timing out on bots. 

http://webkit-test-results.appspot.com/dashboards/flakiness_dashboard.html#tests=editing%2Fdeleting%2Fpassword-delete-performance.html
Comment 10 Alexey Proskuryakov 2013-11-22 09:32:32 PST
*** Bug 79815 has been marked as a duplicate of this bug. ***
Comment 11 Alexey Proskuryakov 2013-11-22 09:36:55 PST
Ryosuke, could you please fix the test? Tracked as bug 124781 now.
Comment 12 Ryosuke Niwa 2013-11-22 18:01:56 PST
(In reply to comment #11)
> Ryosuke, could you please fix the test? Tracked as bug 124781 now.

We should just remove the test.