Open
Bug 918097
Opened 11 years ago
Updated 2 years ago
[mozprofile] If no path for profile given mozprofile should include non-ascii characters into the path
Categories
(Testing :: Mozbase, defect)
Testing
Mozbase
Tracking
(Not tracked)
NEW
People
(Reporter: whimboo, Unassigned)
References
(Depends on 3 open bugs)
Details
Attachments
(1 file)
(deleted),
text/x-review-board-request
|
Details |
As seen on bug 902532 we had a regression in the spellchecker in the final release of Firefox. That happened because some non-ascii characters were included in the profile name.
To prevent those major regressions we should update mozprofile so it automatically adds some non-ascii characters to the profile path by default if the consumer hasn't been specified any location.
Benjamin proposed to have such an option in our automation frameworks. And given that mochitests will be based on mozbase soon (see bug 746243) this would be a great feature.
Will, Jeff already agreed with that proposal via a mail thread, but wanted to get your opinion first. What do you think of it?
Flags: needinfo?(wlachance)
Comment 1•11 years ago
|
||
I'd support at least doing this optionally (you can probably pass it in as an argument to Profile's __init__ method, then down to create_new_profile). At this point, you should be able to make mozmill use that setting.
I'd worry that making this the default might cause problems with *automation* that isn't expecting the non-ascii characters, so I'd prefer to avoid that.
Flags: needinfo?(wlachance)
Reporter | ||
Comment 2•11 years ago
|
||
(In reply to William Lachance (:wlach) from comment #1)
> I'd worry that making this the default might cause problems with
> *automation* that isn't expecting the non-ascii characters, so I'd prefer to
> avoid that.
We might want to do that as the second step given that mozbase is used by mochitests soon. How does it sound to you? Means I would file a follow-up bug and get the optional parameter in for now.
Reporter | ||
Comment 3•7 years ago
|
||
Given all the troubles we have had in the past with profiles containing non-ASCII characters I think we should force using at least one unicode character by default. Also because this will increase the test coverage a lot, additionally to bug 1423897.
We indeed might see issues in automation, but those should only happen due to bugs in Firefox. As I checked with Marionette yesterday and on bug 1438035, our mozbase packages are working just fine with unicode paths.
I think that I will just submit a full try job to check our status.
Ted, what do you think?
Flags: needinfo?(ted)
Comment hidden (mozreview-request) |
Reporter | ||
Comment 5•7 years ago
|
||
As the try job (https://treeherder.mozilla.org/#/jobs?repo=try&revision=684bd5606e3547152b6061f2651b0d28e1c29ce5&selectedJob=162374068) shows a couple of harnesses would need an update to be able to cope with unicode paths.
If my proposal seems worthwhile I can clearly have a look at and fix that.
Reporter | ||
Comment 6•7 years ago
|
||
So if we would make such a change we actually break `mozprofile.diff()` when using the default `diff_function` which is `difflib.unified_diff()`. Reason is that this method is not unicode compatible. At least in Python 2.7.
Comment 7•7 years ago
|
||
I'm not opposed to this idea, and it would increase our test coverage. If this winds up requiring a ton of work on other things then I'm not sure it's worthwhile. If making it the *default* is hard, maybe we could just make our harnesses individually use a profile path with Unicode characters?
Flags: needinfo?(ted)
Reporter | ||
Comment 8•7 years ago
|
||
Lets see what we can do. First I will make sure to make our related mozbase packages unicode safe on Windows. So important in that regard would be mozcrash (bug 1441287). I will add more dependencies when necessary.
Depends on: 1441287
Reporter | ||
Comment 9•7 years ago
|
||
Now that I can run reftests and mochitest locally I pushed another try build to see how test results look on TC:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=1df3f7469bfbcd391e8e1ef5128b24c5b45fe7ba
Updated•2 years ago
|
Severity: normal → S3
You need to log in
before you can comment on or make changes to this bug.
Description
•