Bug 113414

Summary: Rename Editing tests whose expectations are Mac specific
Product: WebKit Reporter: Claudio Saavedra <csaavedra>
Component: New BugsAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: rniwa, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 110487    
Attachments:
Description Flags
Patch
none
Patch none

Description Claudio Saavedra 2013-03-27 08:54:21 PDT
Rename Editing tests whose expectations are Mac specific
Comment 1 Claudio Saavedra 2013-03-27 09:00:58 PDT
Created attachment 195335 [details]
Patch
Comment 2 Ryosuke Niwa 2013-03-27 12:31:12 PDT
Comment on attachment 195335 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=195335&action=review

> LayoutTests/editing/deleting/smart-editing-disabled-mac.html:11
> +if (window.internals)
> +    internals.settings.setEditingBehavior('mac');

Why am I not seeing the diff for this?
Comment 3 Claudio Saavedra 2013-03-27 12:49:55 PDT
Comment on attachment 195335 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=195335&action=review

>> LayoutTests/editing/deleting/smart-editing-disabled-mac.html:11
>> +    internals.settings.setEditingBehavior('mac');
> 
> Why am I not seeing the diff for this?

Because the file is being renamed from smart-editing-disabled.html ? Not sure if that's what you mean.
Comment 4 Ryosuke Niwa 2013-03-27 12:54:36 PDT
(In reply to comment #3)
> (From update of attachment 195335 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=195335&action=review
> 
> >> LayoutTests/editing/deleting/smart-editing-disabled-mac.html:11
> >> +    internals.settings.setEditingBehavior('mac');
> > 
> > Why am I not seeing the diff for this?
> 
> Because the file is being renamed from smart-editing-disabled.html ? Not sure if that's what you mean.

When I do svn mv and then edit a file, I still see a diff after the file is moved. Maybe you're using git? FWIW, you can probably do a partial checkout of just LayoutTests/editing and make changes there.
Comment 5 Claudio Saavedra 2013-03-27 12:59:59 PDT
(In reply to comment #4)
> (In reply to comment #3)
> > (From update of attachment 195335 [details] [details])
> > View in context: https://bugs.webkit.org/attachment.cgi?id=195335&action=review
> > 
> > >> LayoutTests/editing/deleting/smart-editing-disabled-mac.html:11
> > >> +    internals.settings.setEditingBehavior('mac');
> > > 
> > > Why am I not seeing the diff for this?
> > 
> > Because the file is being renamed from smart-editing-disabled.html ? Not sure if that's what you mean.
> 
> When I do svn mv and then edit a file, I still see a diff after the file is moved. Maybe you're using git? FWIW, you can probably do a partial checkout of just LayoutTests/editing and make changes there.

Yes, it's git. But as far as I know when pushing the diff is recorded too (see http://trac.webkit.org/changeset/144883 for example, which also came from git.)
Comment 6 Ryosuke Niwa 2013-03-27 13:02:07 PDT
The point was that I can't easily this patch as is. I'll try to get around to it but I have other work to do this week so you might need to wait for a while.
Comment 7 Ryosuke Niwa 2013-03-27 13:02:21 PDT
Alternatively, you can ask other reviewers to review this.
Comment 8 Claudio Saavedra 2013-03-27 14:00:22 PDT
git diff -M can do this. I think we should probably patch webkit-patch to use the -M modifier.
Comment 9 Ryosuke Niwa 2013-03-27 14:09:10 PDT
(In reply to comment #8)
> git diff -M can do this. I think we should probably patch webkit-patch to use the -M modifier.

Makes sense.
Comment 10 Claudio Saavedra 2013-03-27 14:24:41 PDT
Created attachment 195394 [details]
Patch
Comment 11 Claudio Saavedra 2013-03-27 14:26:40 PDT
Comment on attachment 195394 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=195394&action=review

> LayoutTests/editing/style/push-down-font-styles-mac.html:9
> +<script src="script-tests/push-down-font-styles-mac.js"></script>

Seems like this is guessed wrong.

> LayoutTests/editing/style/push-down-implicit-styles-mac.html:9
> +<script src="script-tests/push-down-implicit-styles-mac.js"></script>

And here as well.
Comment 12 WebKit Review Bot 2013-03-27 17:27:55 PDT
Comment on attachment 195394 [details]
Patch

Clearing flags on attachment: 195394

Committed r147035: <http://trac.webkit.org/changeset/147035>
Comment 13 WebKit Review Bot 2013-03-27 17:27:58 PDT
All reviewed patches have been landed.  Closing bug.