I searched and I did not see this reported, so I figured I would go ahead and file it. According to the W3C validator, links created using Upload are not HTML5 when files have spaces in their names. The validator throws the following exception: "Whitespace in path component. Use %20 in place of spaces."...."Syntax of IRI reference:
Any URL. For example: /hello, #canvas, or http://example.org/. Characters should be represented in NFC and spaces should be escaped as %20."
If you would like to check it out, go ahead and try this validating this page: http://www.croomsalumni.com/about using this validation service http://validator.w3.org/ (just remember to change the DocType to HTML5).
If I am correct, this should be pretty easy to fix, a simple replace function type of deal.
Comment | File | Size | Author |
---|---|---|---|
#40 | 1277140-2.patch | 616 bytes | Everett Zufelt |
#36 | 1277140.patch | 614 bytes | Everett Zufelt |
Comments
Comment #1
frank0051 CreditAttribution: frank0051 commentedJust changing to 6.x-dev
Comment #3
Everett Zufelt CreditAttribution: Everett Zufelt commentedMoving to 8.x as bugs are fixed in dev and backported where appropriate.
Does this happen with image / file fields in Drupal 8?
I am not familiar with the html5 requirements for this, but the validator is experimental and not to be trusted (it doesn't move as quickly as the spec). As far as I can tell this is the requirement http://www.w3.org/TR/html5/urls.html
Comment #4
frank0051 CreditAttribution: frank0051 commentedIs that not what RFC3986 requires - which is supposed to be adhered to in HTML5 (see your link above). Shoot me if I'm off base, haha. Based on my reading (and also what I have seen on other sites), spaces are considered unsafe and must be escaped to meet the standards. Example of such documentation: http://www.tutorialspoint.com/html5/html5_url_encoding.htm
PHP already has a function for it and Stackoverflow touches on it (and the HTML5) requirement here: http://stackoverflow.com/questions/4768262/how-can-i-make-wordpresss-php...
Comment #6
Everett Zufelt CreditAttribution: Everett Zufelt commentedCan someone please confirm that file URLs are still generated with spaces in 8.x?
Comment #7
frank0051 CreditAttribution: frank0051 commentedWell, I don't have 8.0; I only have access to 6 and 7; you are the one that changed it to 8. It appears Drupal 7.x replaces the white space with %20. So, again, this should only be a Drupal 6 problem unless Drupal 8 reverted its handling of files. So, do you want to change it back to a version 6.x?
Comment #8
Everett Zufelt CreditAttribution: Everett Zufelt commentedAs long as we know that head 8.x isn't exhibiting the same issue then we can set it back to 6.x (bugs are fixed in 8.x and then backported where necessary). I assume it is fixed in 8.x if it is fixed in 7.x.
Fixing the bug in 6.x might be difficult. At the most we would want to fix the URL generated for all new uploads, or we * could * break the URLs used / saved that reference the old files.
Comment #9
frank0051 CreditAttribution: frank0051 commentedIs there an 8.x sandbox or something I could test out without having to install it onto my own server? I can try it there if that is a possibility.
Comment #12
Everett Zufelt CreditAttribution: Everett Zufelt commentedTested and 8.x also replaces spaces with properly. So this is a 6.x issue.
Not quite sure the best way to go about solving. For the most part 6.x sites aren't going to be attempting to validate against html5. And, changing the file path for files already uploaded will possibly break existing links.
Ideas?
Comment #13
Damien Tournoud CreditAttribution: Damien Tournoud commentedThe problem is file_create_url() in Drupal 6. We are missing a
drupal_urlencode()
in the FILE_DOWNLOADS_PUBLIC case.Comment #14
frank0051 CreditAttribution: frank0051 commentedWell, I'm not sure Drupal should worry about fixing links that were already uploaded [since it isn't technically a bug]. Rather, lets just make sure that it is fixed for future uploads with D6. Then, if someone else posts on this subject after the patch is included in the next D6 release, individuals can just tell that person to re-upload the files. Just because it is D6 doesn't mean the community shouldn't try its best to future proof it - and, part of that includes readying it for HTML5.
Just my two cents.
Comment #15
Everett Zufelt CreditAttribution: Everett Zufelt commentedTaking a look at theme_upload_attachments(), it appears that files are not stored with their URL, but that the URL is generated from file_create_url() on the theme layer. This means that any changes to file_create_url() to add the encoding would change how all uploaded file URLs are rendered. This might break saved links.
Can someone confirm that a file path encoded with drupal_urlencode() is able to be accessed from the old URL that is not encoded? I expct it can be, and that it shouldn't be a problem to make the change.
Comment #16
frank0051 CreditAttribution: frank0051 commentedIf drupal_urlencode() simply replaces white space with %20 escape characters than it shouldn't matter. Ie9 and Opera already convert whitespace to escaped characters themselves. I imagine the other browsers can handle it as well.
Comment #18
Everett Zufelt CreditAttribution: Everett Zufelt commentedPlease do not 'bump' an issue. This is an open source community with volunteers. I know that for myself if I see an issue getting 'bumped' I am less likely to work on it.
Comment #19
frank0051 CreditAttribution: frank0051 commentedWell, unfortunately, I have found some issues go weeks or even months without reply if they aren't bumped. While I understand your point of view and appreciate all the hard work volunteers put into the project, sometimes I find it helps to just bring this back to the top of people's list. Sometimes people get busy and overlook things.
Comment #20
Damien Tournoud CreditAttribution: Damien Tournoud commentedI described what we need to fix in #13. Now we need someone to contribute a patch!
See http://drupal.org/patch/create for more information.
Comment #21
frank0051 CreditAttribution: frank0051 commentedDamien, I'll give it a try in the next hour or two; I've never done one before and I might break it, so you guys will have to check it, haha. I'm just layman.
Comment #22
frank0051 CreditAttribution: frank0051 commentedReading through the directions, I'm definitely not comfortable with trying to create a patch. But here is what I think needs to happen.
In includes/file.inc, there is this function
I think you just need to add one line of code (bolded)
function file_create_url($path) {
// Strip file_directory_path from $path. We only include relative paths in urls.
if (strpos($path, file_directory_path() .'/') === 0) {
$path = trim(substr($path, strlen(file_directory_path())), '\\/');
}
$path=drupal_urlencode($path);
switch (variable_get('file_downloads', FILE_DOWNLOADS_PUBLIC)) {
case FILE_DOWNLOADS_PUBLIC:
return $GLOBALS['base_url'] .'/'. file_directory_path() .'/'. str_replace('\\', '/', $path);
case FILE_DOWNLOADS_PRIVATE:
return url('system/files/'. $path, array('absolute' => TRUE));
}
}
Comment #23
frank0051 CreditAttribution: frank0051 commentedAnyone think they can develop a patch using the suggestion I made above?
Comment #27
cweagansInstead of continually bumping the issue, why don't you develop a patch? There's a first time for everything, this is an easy change, and documentation for creating a patch was already posted.
And seriously, don't bump issues like that. It just annoys people. If you want it done quicker, do it yourself or hire someone to do it for you.
Comment #29
frank0051 CreditAttribution: frank0051 commentedI spent 8 hours attempting to do simple CSS over the last two days, you really don't want me to attempt to develop a patch. And, I'm not a developer. I realize this is an open source community, so I'm contributing in what ways I can. I have brought the issue to your attention and I laid out exactly where you need to insert the ONE line of code that I think is needed. All you have to do is "patch-ify" it and test it.
And, I'm going to keep bumping when something doesn't get a reply for over a week or two. The last time I bumped before the most recent one was 23 days before that. I'm regularly used to having Drupal issues go unsolved, so I'm trying something new. It's actually working better than I had hoped. Now, if I were an ass, I would say why don't you do it sense you appear to know how instead of lecturing me. I'm willing to bet you could have done it in the time you spent lecturing me. Meanwhile, I would be taking 7 hours just to figure out how to draw a CSS box.
Comment #30
cweagansYou misunderstand how this works. I don't have to do anything. If this is affecting you, then do something about it. Read the patch documentation posted in #20, follow the directions, and post a patch. The worst thing that can happen is that it doesn't work. There was a first time for all of us, but the important thing is that you try. Plus, it helps that you've already done all the hard work (figuring out what code needs to go where).
Unless it's a critical issue (which this is not), it's extremely bad form to bump issues. It might be working for you, but it's just because people get annoyed enough to comment on the issue again. The entire reason that the "Follow" button was implemented on Drupal.org was that people were tired of one-word "subscribe" comments: they didn't add anything but noise to the discussion. Bump comments are along the same lines. Plus, it comes off as "I want somebody else to fix this for me". While that may be the case, we're all volunteers and most of us don't like being told what we're going to work on. The only reason I saw this post in the first place was that somebody (I won't say who) was annoyed with you on Twitter.
I'm unsubscribing from this thread now. If you'd like to try your hand at contributing a patch, feel free to reach out to me or anyone on this issue via the Drupal IRC channels: I'm certain that if you say "I'm interested in making a patch for issue 1277140, can somebody walk me through the process?" on IRC, you'll get lots of people lining up to help you. Good luck.
Comment #31
frank0051 CreditAttribution: frank0051 commentedThat was a YOU as YOU the community. Context clues buddy. And, no, I don't have another 7 hours to exhaust like I did over Thanksgiving Break. Perhaps you may have that type of time, but I work 55-60 hour weeks along with managing the various side projects that take up another 15-25 hours in any given week.
Damn straight I want someone else to fix it. I've identified the issue, provided the code, now I want some experienced people to write it and help test it in order to not mess it up. Open source communities seem to be behind the curve when realize not everyone that uses their software is a developer and some people would like some good ol' fashion customer support, but will contribute where they can to the cause.
Comment #33
chx CreditAttribution: chx commentedfrank0051, at your next bump
Iwe will ban you. You will not receive any more warnings. Do not answer to this comment.Edit: I deleted all your bumps from this and other issues.
Comment #34
frank0051 CreditAttribution: frank0051 commentedSee my reply to your issue and maybe you will understand why: http://drupal.org/node/1374846#comment-5378108
This thread has been open since September. The solution has basically when proposed, all I need someone to do is "patch-ify" it. That's literally all that remains. The other issue (JS for Opera) is literally a core issue that has made my three installs of Drupal useless because Javascript doesn't work properly in the core of 6.x.
Comment #35
frank0051 CreditAttribution: frank0051 commentedI will pay anyone, right now, $20 to patch-ify that solution in this thread and help test it. I've already done the heavy-lifting when it comes to figuring out what the problem is and IDing the PHP code.
Comment #36
Everett Zufelt CreditAttribution: Everett Zufelt commentedComment #37
frank0051 CreditAttribution: frank0051 commentedI'll play with this tonight. Just find it funny that this is something that was probably 30 minutes of work for you (especially since you have already commented on this thread), and it wasn't the fact that this was a core usability issue that motivated you, but rather money. That's a sad statement on the open source community that has developed here at Drupal. In any case, I'm a man of my word; message me your Pay-Pal address and if this works I'll send you the money.
Comment #38
Everett Zufelt CreditAttribution: Everett Zufelt commented@Frank
Although your comment is off topic, and a little insulting, I will comment here.
As a contributor ranked by CertifiedToRock.com as being in the 99th percentile, and a contributor to other communities as well, like the HTML WG, I give far more than the average individual back to the open source world. Nevertheless, the last time I asked my landlord if I could get credit toward rent by showing a commit log, he said no.
Furthermore, I did not roll this patch for money, although I'll take that, but because I thought that this was evidence of you actually showing respect to the community process (i.e. do it yourself, wait for someone to do it for free, or pay someone to do it for you).
Comment #39
frank0051 CreditAttribution: frank0051 commentedAgain, I go back to this is a CORE USABILITY ISSUE. The whole point is it shouldn't have taken over 4 months to get solved, it shouldn't have resulted in a threat to ban me, and it shouldn't have required money to solve. Are you individually to blame, of course not, but the community as a whole (myself included) should be ashamed that a core issue like this could go outstanding for so long. And, BTW, Drupal is still difficult to Opera users because of the JS error in the other thread I have mentioned.
That being said, the patch does not appear to work correctly. Instead of generating http://www.croomsalumni.com/sites/default/files/Historical%20Listing%20o...
it generates
http://www.croomsalumni.com/http%253A/%25252Fwww.croomsalumni.com/sites/...
Thank you for your attempt Everett, but unfortunately I can not pay for a solution that does not work.
Comment #40
Everett Zufelt CreditAttribution: Everett Zufelt commentedGoes to show you should read the docs before writing a patch.
Comment #41
frank0051 CreditAttribution: frank0051 commentedWorks perfect, I'll send payment in 5 minutes. How do we get it committed?
Comment #42
Everett Zufelt CreditAttribution: Everett Zufelt commented@Frank
There isn't really a hard and fast guideline. Normally a couple people, who didn't write the patch, review the patch. If it is found to be working and not causing any problems someone sets the status to RTBC. This is a pretty straight forward patch, literally modifying a single line, wih highly predictable results, so it would be little work for another person to give a quick review.
Once the issue status is RTBC someone with permission to commit to the 6.x branch will take a look, and if they agree they will apply the patch and commit the changes. It will then be part of the next Drupal 6.x release.
As this is an issue that clearly very few people have found important then I don't expect to see that happening immediately, but I expect that it will happen in a reasonable timeframe.
Comment #43
cweagansPretty straightforward.
Comment #44
Damien Tournoud CreditAttribution: Damien Tournoud commentedThe patch matches my spec from #13. I support committing to 6.x. As previously stated, 7.x and 8.x are not affected.
Comment #45
hass CreditAttribution: hass commentedI have tested this patch and it fixes #1525146: Space in filename triggers 404. Thanks for sharing. RTBC.
Why is this patch not committed to D6 after 1 year of being RTBC? Bumping for commit.
Comment #46
hass CreditAttribution: hass commentedMarking as release blocker for next D6.
Comment #47
hass CreditAttribution: hass commented2 years bump. Is Drupal really maintained?
Comment #49
asb CreditAttribution: asb commentedCould someone please update the status of this issue? Is it RTBC, as #45 reports, or is it not, as #48 suggests?
Comment #51
hass CreditAttribution: hass commentedJust a broken testbot, nothing else.