convert editing/deleting/paragraph-in-preserveNewline.html to dumpAsText
Created attachment 98555 [details] Patch
Comment on attachment 98555 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=98555&action=review > LayoutTests/editing/deleting/paragraph-in-preserveNewline-expected.txt:14 > + > +bar We should use dump-as-markup so that we can see the markup in #test. r- due to loss of test coverage.
Created attachment 98770 [details] Patch
Comment on attachment 98770 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=98770&action=review > LayoutTests/ChangeLog:11 > + * editing/deleting/paragraph-in-preserveNewline-expected.txt: Added. I don't see this file in the patch.
Created attachment 98862 [details] Patch
Comment on attachment 98770 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=98770&action=review >> LayoutTests/ChangeLog:11 >> + * editing/deleting/paragraph-in-preserveNewline-expected.txt: Added. > > I don't see this file in the patch. That's very odd - I should have caught that it was missing! I looked at a diff of the branch from earlier, and it shows my edits; somehow the script didn't recognize it. Anyway, new patch (with results) is attached.
Comment on attachment 98862 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=98862&action=review > LayoutTests/editing/deleting/paragraph-in-preserveNewline-expected.txt:6 > +| <pre> > +| "<#selection-caret> > +bar" Huh, bar is outside of pre? Please file a bug for this. > LayoutTests/editing/deleting/paragraph-in-preserveNewline.html:20 > +if (window.layoutTestController) > + layoutTestController.dumpEditingCallbacks(); You need to do this before modifying selection & calling execCommand("Delete"). r- because of this.
Created attachment 98956 [details] Proposed patch Hopefully this works - I manually made the patch file using 'git diff -M100 commit-number'. The upload script generates a diff somewhere, and git's dumb "smart rename tracking" which only comes to the surfaces when generating diffs causes the file not to be shown. Passing -M100 or --no-renames to the diff line keeps it from finding renames, which is great - but modifying the upload script to include either of these in calls to git diff didn't seem to have an effect. Anyway, since webkit-patch doesn't have the ability to pretty-preview an arbitrary patch from a file, and I don't fully trust my ability to read raw diffs, I'll know after uploading this whether the manual diff worked quite the way I wanted.
Comment on attachment 98862 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=98862&action=review >> LayoutTests/editing/deleting/paragraph-in-preserveNewline-expected.txt:6 >> +bar" > > Huh, bar is outside of pre? Please file a bug for this. bar is in a string which has a \n in it -- so it appears on the next line without a new |
Created attachment 98957 [details] Proposed patch Trying again with '--binary' in the git diff line.
Comment on attachment 98862 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=98862&action=review >>> LayoutTests/editing/deleting/paragraph-in-preserveNewline-expected.txt:6 >>> +bar" >> >> Huh, bar is outside of pre? Please file a bug for this. > > bar is in a string which has a \n in it -- so it appears on the next line without a new | Ah ok.
The attached patch works - I reapplied it to a clean local dir successfully. For future reference, 'git diff -M100 --binary commit-id > patch.diff' is what to use. I'm going to look into the scripts and see if I can get them to mimic this; it would be cool to have git do renames properly for our purposes.
Comment on attachment 98957 [details] Proposed patch Clearing flags on attachment: 98957 Committed r90031: <http://trac.webkit.org/changeset/90031>
All reviewed patches have been landed. Closing bug.