[Webkit-unassigned] [Bug 135929] import-w3c-tests doesn't handle relative paths to support files in ref files correctly

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=135929


Bem Jones-Bey <bjonesbe at adobe.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #237605|review?                     |review-
               Flag|                            |




--- Comment #3 from Bem Jones-Bey <bjonesbe at adobe.com>  2014-09-04 11:06:58 PST ---
(From update of attachment 237605)
View in context: https://bugs.webkit.org/attachment.cgi?id=237605&action=review

> Tools/ChangeLog:8
> +	The recent refactor of the W3C test repo falsified a bunch of assmumptions that

Fix the indentation here. (The style bot also complained. :-)

> Tools/Scripts/webkitpy/w3c/test_converter.py:40
> +def convert_for_webkit(new_path, filename, refrelpath, host=Host()):

Nit: I don't really like the name refrelpath. I think for consistency with resources_relpath below, this should at least be ref_relpath. reference_relpath would be better, I think. (I'd really like refernce_relative_path, but relpath is used in a bunch of places already, so probably best to stick with that for now.) It's up to you if you change it.

> Tools/Scripts/webkitpy/w3c/test_converter.py:174
> +                if attr_name == 'src' and  not attr_value.startswith('data:'):

Do you ever want to replace if you have a URL scheme at all? For example, if you have http: or file:, you probably don't want to do the replacement, either.

Also, shouldn't you be checking the scheme in the case of a url() in convert_relative_reffile_paths()? It seems like a helper function to actually do the replacement (or not if you have a path that doesn't need a replacement) would be a good idea.

> Tools/Scripts/webkitpy/w3c/test_parser.py:103
> +                    test_info['refrelpath'] = self.filesystem.relpath(self.filename, ref_file).replace('../', '', 1).replace(testfile[1], '')

Why do you remove the first '../'?

> Tools/Scripts/webkitpy/w3c/test_parser.py:-137
> -        """ Searches the file for all paths specified in url()'s, href or src attributes."""

Why did you remove the search for href attributes?

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.