[webkit-reviews] review denied: [Bug 45791] NRWT fails with UnicodeDecodeError on editing/selection/mixed-editability-10.html : [Attachment 67626] Patch

[bugzilla-daemon at webkit.org]

Eric Seidel <eric at webkit.org> has denied  review:
Bug 45791: NRWT fails with UnicodeDecodeError on
editing/selection/mixed-editability-10.html
https://bugs.webkit.org/show_bug.cgi?id=45791

Attachment 67626: Patch
https://bugs.webkit.org/attachment.cgi?id=67626&action=review

------- Additional Comments from Eric Seidel <eric at webkit.org>
View in context:
https://bugs.webkit.org/attachment.cgi?id=67626&action=prettypatch

> WebKitTools/Scripts/webkitpy/layout_tests/port/base.py:160
> +	   expected_filename = str(expected_filename)
> +	   actual_filename = str(actual_filename)
This will take expected_filename and try to decode it as ascii.  Is that what
you intended?  If expected_filename is already a str() object it has no effect,
but if it's unicode() that's effectively calling filename.encode('ascii') which
may not be what you want?  (At least that's my understanding.)

> WebKitTools/Scripts/webkitpy/layout_tests/port/base_unittest.py:175
> +	   port.diff_text('exp', 'act', 'exp.txt', 'act.txt')
> +	   port.diff_text('exp', 'act', u'exp.txt', 'act.txt')
> +
> +	   port.diff_text('exp' + chr(255), 'act', 'exp.txt', 'act.txt')
> +	   port.diff_text('exp' + chr(255), 'act', u'exp.txt', 'act.txt')
Seems we need a test with unicode characters here.

I don't think this is 100% right, but please correct me if I'm wrong. :)