645002 - Don't allow user to select a non-writable storage location

Status: RESOLVED FIXED

(Mozilla QA Graveyard :: Mozmill Crowd Extension, defect)

Description

[[:Aleksej]]

Tests in Mozmill Crowd 0.1.2 fail silently if the specified storage directory is on a volume with noexec (like /tmp is supposed to be, according to some — although not according to Debian).

Comment 1

[Henrik Skupin [:whimboo][⌚️UTC+2]]

That applies to any platform, if the storage folder is not writable. Should be easy to fix for the next release. We will triage that later.

Comment 2

[[:Aleksej]]

noexec means files in that directory cannot be executed (the "x" permission on files is ignored)

Comment 3

[Henrik Skupin [:whimboo][⌚️UTC+2]]

We should include this bug in v0.2. Has someone interests to work on it? It shouldn't be that hard to get fixed.

Comment 4

[Aaron Train [:aaronmt]]

Attachment: Writable check

+ Checks if the specified storage location is writable or not

Comment 5

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Comment on attachment 529108 [details] [diff] [review]
Writable check

>+          window.alert(this._stringBundle.getString("storage.path_isNotWritable")); 

We should be consistent with the naming of properties. For now we are using underscores to separate the words from each other. We should probably revise that but for now please don't use camel case. "path_not_writable" would be fine.

>+storage.path_isNotWritable=The specified folder must have writable permissions. Please select a different folder.

"writable" is not a common word I have seen in failure messages so far. Can you use "read-only" instead?

Comment 6

[Aaron Train [:aaronmt]]

Attachment: Writable check - v2

+ Reworded property and property definition based on review comment above

Comment 7

[[:Aleksej]]

(In reply to comment #6)
> The specified folder can not be read-only. Please select a different folder.

I think it should be “is” instead of “can not be”. It will be clear that it should not be read-only. Otherwise it sounds like the problem is that the folder actually specified can not be made read-only.

Comment 8

[Aaron Train [:aaronmt]]

(In reply to comment #7)
> (In reply to comment #6)
> > The specified folder can not be read-only. Please select a different folder.
> 
> I think it should be “is” instead of “can not be”. It will be clear that it
> should not be read-only. Otherwise it sounds like the problem is that the
> folder actually specified can not be made read-only.

If that we're the case, 'is' does not reference the point of issue and and an act of change. By using 'can not' or 'must not', it implies an action of change.

Comment 9

[u415141]

A Pivotal Tracker story has been created for this Bug: https://www.pivotaltracker.com/story/show/13229209

Comment 10

[u415141]

Henrik Skupin changed story state to finished in Pivotal Tracker

Comment 11

[u415141]

Henrik Skupin deleted the linked story in Pivotal Tracker

Comment 12

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Comment on attachment 529465 [details] [diff] [review]
Writable check - v2

>+storage.path_not_writable=The specified folder can not be read-only. Please select a different folder.

I have to agree with Aleksej, the first sentence is kinda hard to understand. It's probably kinda hard to formulate the right sentence with 'read-only' only.

Why can't we simply say: "The specified folder is 'read-only' but needs write permissions. Please ..."?

Comment 13

[Aaron Train [:aaronmt]]

Attachment: Writable check - v3

"The specified folder is read-only, it must have a write permission. Please select a different folder."

Comment 14

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Comment on attachment 532038 [details] [diff] [review]
Writable check - v3

>+storage.path_not_writable=The specified folder is read-only, it must have a write permission. Please select a different folder.

Could we agree on "The specified folder is read-only, but must have write permission"? I have never seen the usage of 'a' in any article and it reads strange that way. And with 'but' we can explicitly point out what is necessary.

Comment 15

[Aaron Train [:aaronmt]]

Attachment: Writable check - v4

"The specified folder is read-only but must have write permission. Please select a different folder."

Comment 16

[Aaron Train [:aaronmt]]

Attachment: Writable check - v4

Can't forget conjunction comma now can we

Comment 17

[Aaron Train [:aaronmt]]

Landed as https://github.com/whimboo/mozmill-crowd/commit/7ce9480ee561472caa40c7f34431c1aa70501612

Comment 18

[Henrik Skupin [:whimboo][⌚️UTC+2]]

We missed the noexec part of this bug. Sorry for that. Alex please file a new bug for it.

Comment 19

[[:Aleksej]]

(In reply to comment #18)

Filed bug 668752.