WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
57872
REGRESSION (
r46914
,
r48764
): When typing in Mail, line wrapping frequently occurs in the middle of words
https://bugs.webkit.org/show_bug.cgi?id=57872
Summary
REGRESSION (r46914, r48764): When typing in Mail, line wrapping frequently oc...
Adele Peterson
Reported
2011-04-05 12:36:05 PDT
Created
attachment 88293
[details]
testcase <
rdar://problem/8157205
>
http://trac.webkit.org/changeset/46914
was not supposed to change behavior, but it definitely did. From looking of different reports of this problem, it seems to frequently happen with tab characters. I'm attaching an example that shows the problem. Before
r46914
, when you hit enter after the tab character and start typing, the new text doesn't get the "white-space: pre" style, but after that change, it does. To fix Mail, if we changed the tab style to use "pre-wrap", that would fix this problem, but I'd like to figure out what went wrong in
r46914
, and whether we can go back to the old behavior.
Attachments
testcase
(755 bytes, text/html)
2011-04-05 12:36 PDT
,
Adele Peterson
no flags
Details
fixes the bug
(8.55 KB, patch)
2011-04-06 05:49 PDT
,
Ryosuke Niwa
no flags
Details
Formatted Diff
Diff
work in progress (applies on top of the original patch)
(4.47 KB, patch)
2011-04-06 17:48 PDT
,
Ryosuke Niwa
no flags
Details
Formatted Diff
Diff
complete patch (needs to rebaseline editing/inserting/6633727.html)
(17.90 KB, patch)
2011-04-07 08:02 PDT
,
Ryosuke Niwa
no flags
Details
Formatted Diff
Diff
fixes the bug
(18.57 KB, patch)
2011-04-07 13:20 PDT
,
Ryosuke Niwa
no flags
Details
Formatted Diff
Diff
Show Obsolete
(3)
View All
Add attachment
proposed patch, testcase, etc.
Ryosuke Niwa
Comment 1
2011-04-06 03:28:12 PDT
This bug was a regression from two change sets.
http://trac.webkit.org/changeset/46914
initially introduced a regression because it wasn't avoiding tab span to obtain the computed style to apply after the insertion.
http://trac.webkit.org/changeset/46914
introduced another regression by copying hierarchy under new block at the insertion position without avoiding the tab span. I'm going to upload a patch in a minute to fix this bug, but we should really get rid of this tab span business at some point.
Ryosuke Niwa
Comment 2
2011-04-06 03:28:45 PDT
I mean to say "
r48764
introduced another regression".
Ryosuke Niwa
Comment 3
2011-04-06 05:49:34 PDT
Created
attachment 88401
[details]
fixes the bug
Eric Seidel (no email)
Comment 4
2011-04-06 05:53:22 PDT
Comment on
attachment 88401
[details]
fixes the bug Seems reasonable.
Ryosuke Niwa
Comment 5
2011-04-06 05:55:27 PDT
(In reply to
comment #4
)
> (From update of
attachment 88401
[details]
) > Seems reasonable.
Thanks for the review! Landing it now.
Ryosuke Niwa
Comment 6
2011-04-06 05:59:40 PDT
Committed
r83039
: <
http://trac.webkit.org/changeset/83039
>
WebKit Review Bot
Comment 7
2011-04-06 06:54:05 PDT
http://trac.webkit.org/changeset/83039
might have broken Windows 7 Release (Tests)
Adele Peterson
Comment 8
2011-04-06 13:03:53 PDT
I believe this caused a serious regression, and may need to be rolled out. Running Mail with this change, use the following steps: - Set a default signature - Place the caret at the beginning of the message - Hit tab - Copy a url from Safari's address field and paste it into the message - Hit Enter The caret will skip over the signature and land at the end of the message
Ryosuke Niwa
Comment 9
2011-04-06 13:22:10 PDT
(In reply to
comment #8
)
> Running Mail with this change, use the following steps: > > - Set a default signature > - Place the caret at the beginning of the message > - Hit tab > - Copy a url from Safari's address field and paste it into the message > - Hit Enter > > The caret will skip over the signature and land at the end of the message
Mn... that seems to imply that the pasted URL is inserted inside a tab span because all this patch does is to avoid inserting a paragraph separator inside a tab span. Do you happen to know the HTML right before you hit the enter?
Adele Peterson
Comment 10
2011-04-06 13:31:37 PDT
Yes, you're right that the url gets pasted in the tab-span. <html><head></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space; "><span class="Apple-tab-span" style="white-space:pre">
https://bugs.webkit.org/show_bug.cgi?id=57872
</span><br><div id="AppleMailSignature"> <span class="Apple-style-span" style="border-collapse: separate; color: rgb(0, 0, 0); font-family: Helvetica; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: 2; text-align: -webkit-auto; text-indent: 0px; text-transform: none; white-space: normal; widows: 2; word-spacing: 0px; -webkit-border-horizontal-spacing: 0px; -webkit-border-vertical-spacing: 0px; -webkit-text-decorations-in-effect: none; -webkit-text-size-adjust: auto; -webkit-text-stroke-width: 0px; font-size: medium; "><div>Thanks,</div><div><span class="Apple-tab-span" style="white-space: pre; "> </span>Adele</div></span> </div> <br></body></html>
Ryosuke Niwa
Comment 11
2011-04-06 13:48:00 PDT
I'm rolling out
http://trac.webkit.org/changeset/83039
for now per Adele's request. Will investigate the regression tomorrow.
Ryosuke Niwa
Comment 12
2011-04-06 17:48:14 PDT
Created
attachment 88549
[details]
work in progress (applies on top of the original patch) Here's working in progress. I'm reverting
r48764
for the general path and instead solving the
bug 29740
(a.k.a.
rdar://problem/7168738
) by splitting nodes up until the startBlock.
Ryosuke Niwa
Comment 13
2011-04-07 08:02:56 PDT
Created
attachment 88639
[details]
complete patch (needs to rebaseline editing/inserting/6633727.html)
Ryosuke Niwa
Comment 14
2011-04-07 08:04:11 PDT
Comment on
attachment 88639
[details]
complete patch (needs to rebaseline editing/inserting/6633727.html) Here's my latest work in progress. I can upload the final patch for review as soon as the
bug 58037
is fixed.
Ryosuke Niwa
Comment 15
2011-04-07 13:20:07 PDT
Created
attachment 88679
[details]
fixes the bug
Ryosuke Niwa
Comment 16
2011-04-07 13:23:50 PDT
This patch hopefully fix all the regressions and bugs we have.
Darin Adler
Comment 17
2011-04-07 13:32:45 PDT
Comment on
attachment 88679
[details]
fixes the bug Looks like a good approach.
Ryosuke Niwa
Comment 18
2011-04-07 13:35:39 PDT
Comment on
attachment 88679
[details]
fixes the bug (In reply to
comment #17
)
> (From update of
attachment 88679
[details]
) > Looks like a good approach.
Thanks for the review. Hopefully, this won't introduce a new regression...
WebKit Commit Bot
Comment 19
2011-04-07 21:08:01 PDT
The commit-queue encountered the following flaky tests while processing
attachment 88679
[details]
: http/tests/websocket/tests/multiple-connections.html
bug 53825
(author:
abarth@webkit.org
) The commit-queue is continuing to process your patch.
WebKit Commit Bot
Comment 20
2011-04-07 21:11:34 PDT
Comment on
attachment 88679
[details]
fixes the bug Clearing flags on attachment: 88679 Committed
r83247
: <
http://trac.webkit.org/changeset/83247
>
WebKit Commit Bot
Comment 21
2011-04-07 21:11:39 PDT
All reviewed patches have been landed. Closing bug.
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug