Make WordPress Core

Opened 15 years ago

Last modified 20 months ago

#9716 reopened defect (bug)

WordPress Theme/Plugin editor adds blank lines

Reported by: vistronic's profile Vistronic Owned by:
Milestone: Future Release Priority: normal
Severity: normal Version: 2.7.1
Component: Administration Keywords: has-patch needs-testing
Focuses: Cc:

Description (last modified by SergeyBiryukov)

When I edit theme css in theme editor in WP admin
WordPress reformats my CSS and adds a blank life between entrys in css.
it adds a cr and line break ....
In other words if you have a 100 line css
download with FTP look at in text editor
it is now 200 lines
as WP added blank lines to css
it should not be adding things to the CSS?

It may only do this when it wraps window but I think not it appears to add blank lines.

Imagine the damage to a 1000 line CSS it spaces every entry by 1 line!!!!!!!!!!!!!! It stiil works but it messes it up.

THAT IS COFIRMED ONE EDIT DOUBLE SPACE ENTIRE CSS... VERY BAD

Here is part of the problem but not the double space in total.
When you down load the file in FTP and it is in windows encoding
the line endings.
So you ftp back and all is fine.
Now you are at the coffee shop and what a quick change, so you login to WP admin and theme editor edit CSS.
Now you get home and download in ftp to continue your work on CSS.
The file is now mac formatted (or thinks it is) so when you edit the line endings do not contain the right line breaks for windows OR WP theme editor as it ignores them. This explains the wierd spacing though not the double spacing .... why is it converting to mac and yet ignores mac?!
weird.. it adds a CR tag
must be bug
again though its still double spacing on edit IN windows line ending though if you down load cSS its tagged mac format...

Use notepad 2 or Note pad plus to see the error.
When ever you edit with WordPress editor it adds cr and says css is now mac formatted.

FTP download the file and now see all the extra CR's it adds.

So it likes CR only so I convert to CR only and it beaks CSS it wants CR LF to work but if that is the case why does the online editor add CR only?????????????????????????

Attachments (2)

9716.patch (4.2 KB) - added by kurtpayne 12 years ago.
Give the user option of line endings
9716.2.patch (1.2 KB) - added by SergeyBiryukov 12 years ago.

Download all attachments as: .zip

Change History (32)

#1 @azaozz
15 years ago

  • Resolution set to invalid
  • Status changed from new to closed

WordPress 2.7.1 doesn't have CSS editor, it has file editing capabilities using a textarea where everything is controlled by your browser. If you're having problems try another browser or file a bug with the browser's creators.

If you were testing WordPress 2.8 which includes CodePress, the proper place for bug reports would be CodePress' bug tracker.

#2 @Denis-de-Bernardy
15 years ago

  • Milestone Unassigned deleted

#3 @harmr
12 years ago

  • Cc harmr added
  • Component changed from General to Editor
  • Keywords needs-refresh needs-testing added; CSS removed
  • Resolution invalid deleted
  • Status changed from closed to reopened
  • Summary changed from Wordpress Theme editor adds CR tag and Mac format CSS file to Wordpress Theme/Plugin editor adds blank lines
  • Version changed from 2.7.1 to 3.2.1

The reported problem still exists´(and isnt limited to editing css files). I´ve been struggling for years with this bug and havent found a solution yet. When saving theme or plugin files via Wordpress backend editor, blank lines are added after each line - which on the other hand are only seen, if I open the file via Dreamweaver or ftp/download. I then have to do a reg-ex-search and replace to get them removed (find: [\r\n]{2,} replace with: \n )

#4 @duck_
12 years ago

  • Keywords editor needs-refresh needs-testing removed
  • Milestone set to Awaiting Review

Moving to Awaiting Review as the blank milestone is putting this in report/5 (i.e. for 3.3).

@kurtpayne
12 years ago

Give the user option of line endings

#5 @kurtpayne
12 years ago

  • Cc kpayne@… added
  • Keywords has-patch added

#6 @kurtpayne
12 years ago

  • Keywords reporter-feedback added

9716.patch may not undo the "double line" problem you're seeing, harmr. The file is read off the file system (using file_get_contents), presented in the browser (very dependent upon your browser + OS), then posted back to the server, and written back using fopen/fwrite.

Somewhere in that process, the line endings are being translated incorrectly. WordPress is using common practices for working with text files in the web, but this patch should give you a choice as to how the line endings are translated when they written back to the server. The fopen mode in the patch has been changed, too, to prevent any unexpected EOL translation.

There's some conflicting notes on the fopen documentation page about using 't' and 'b' modes depending on your goals.

Please report back if this helps resolve the problem going forward, introduces new problems, undoes the old "double line" problem in existing files, etc.

#7 follow-up: @SergeyBiryukov
12 years ago

  • Version changed from 3.2.1 to 2.7.1

Version number indicates when the bug was initially introduced/reported.

Replying to harmr:

When saving theme or plugin files via Wordpress backend editor, blank lines are added after each line - which on the other hand are only seen, if I open the file via Dreamweaver or ftp/download.

Sounds like Dreamweaver doesn't autodetect EOL properly. Could you describe the steps (including environment) to reproduce the addition of blank lines?

#8 @helenyhou
12 years ago

#20244 closed as duplicate.

#9 in reply to: ↑ 7 @azaozz
12 years ago

Replying to SergeyBiryukov:

Sounds like Dreamweaver doesn't autodetect EOL properly...

Seems line breaks have to be normalized when saving edited files. Also if line breaks are \r, the theme or plugin "header" becomes inaccessible. This is browser and OS dependent but seems all modern browsers work with \n line breaks in a textarea. So the patch can be as simple as:

srt_replace( array("\r\n", "\r"), "\n", $content );

Related #20248

Last edited 12 years ago by azaozz (previous) (diff)

#10 @nacin
12 years ago

When we do that, we can cause problems if someone then opens up the file on their local OS.

If someone were to open a \n-normalized file in Notepad, when it was originally \r\n, it's a bad experience. I dealt with this in GSoC two years ago but didn't come up with a great solution. We need to make sure the file displays fine but saves with the existing endings.

#11 @mbijon
12 years ago

  • Cc mike@… added

#12 @HallMarc
12 years ago

  • Cc HallMarc added
  • Keywords needs-patch added; has-patch removed

I have found a way to normalize the contents before they get written back to the file. In both wp-admin/theme-editor.php and wp-admin/plugin-editor.php you need to change one line of code:

Find:

	$newcontent = stripslashes($_POST['newcontent']);

And replace it with this:

	$newcontent = ereg_replace("\r\n?", "\n", stripslashes($_POST['newcontent']));

And all is well.
I hope this gets included in the next update.
Since I am unsure as of yet as to why the extra CR gets added or when; the ability for the end-user to turn this on/off may be a good addition.

Last edited 12 years ago by HallMarc (previous) (diff)

#13 @HallMarc
12 years ago

  • Keywords needs-patch removed

@SergeyBiryukov - Thank you for updating what I posted to a non-deprecated function. Although IMHO I would use preg-replace instead of str-replace as the first is for reg exp.

	$newcontent = preg_replace("(\r\n|\r|\n\r)","\n",stripslashes($_POST['newcontent']));

@Everyone - Curious if this will be the one fix we need. Both solutions have worked for me without causing any funkiness when opening the file after a WP edit in any program; Dreamweaver, Sublime Text 2, Windows Notepad and WordPad included. If this patch works across all platforms *nix/Windows then we won't need the toggle option.

Last edited 12 years ago by HallMarc (previous) (diff)

#14 @SergeyBiryukov
12 years ago

  • Description modified (diff)
  • Keywords has-patch needs-testing added; reporter-feedback removed
  • Milestone changed from Awaiting Review to Future Release
  • Summary changed from Wordpress Theme/Plugin editor adds blank lines to WordPress Theme/Plugin editor adds blank lines

#15 follow-up: @nacin
12 years ago

I like kurtpayne's patch, 9716.diff, but rather than an option, we should just detect the line endings of the current file, normalize them on display, and then save them based on the original detection.

#16 @HallMarc
12 years ago

@nacin I hear you and I have to say that even when I explicitly set the fopen to preserve the EOL in the original file it still ends up with extra EOL's in the document. This occurs with each and every update too; at least in my experience so far. This is why I am backing the patch that "normalizes" the string before writing it to the file. I know this is isn't the most elegant solution, yet....

#17 in reply to: ↑ 15 ; follow-up: @kurtpayne
12 years ago

Replying to nacin:

we should just detect the line endings of the current file, normalize them on display, and then save them based on the original detection.

What's the best way to detect line endings? A file could have mixed line endings throughout. Here's an edge case:

  1. User 1 uploads a file with DOS line endings
  2. User 2 edits the file in vi (^M all over the place), makes a few surgical edits (now some unix line endings in the file), re-saves it
  3. User 3 edits the file through WP

How does WordPress know how to restore the original line endings when User 3 saves the file?

#18 in reply to: ↑ 17 @azaozz
12 years ago

Replying to kurtpayne:

How does WordPress know how to restore the original line endings...

And what are the original line endings anyways? Most core files have \n EOLs, seems many plugins have \r\n EOL and themes generally have \n EOL (depending on the OS preferences of the authors).

Thinking we should standardize on either one or the other. A good criteria would probably be to determine on which platform non-native EOLs would cause more problems. i.e. Notepad on Windows vs. TextEdit on MacOS. As far as I can see TextEdit "understands" \r\n EOLs and Notepad doesn't understand \n, so \r\n seems as the proper default.

#19 follow-up: @nacin
12 years ago

Standardizing on \r\n is really lame for anyone not on Windows. We should try *very* hard to avoid modifying line endings. People might use the theme editor for a quick change and this could have some bad effects to workflows.

If the file has a mix of line endings, then sure, we just choose a default. But we can be smart about it.

HallMarc, please upload here a file you are struggling with. I can't reproduce.

#20 in reply to: ↑ 19 @azaozz
12 years ago

Replying to nacin:

Standardizing on \r\n is really lame for anyone not on Windows.

Seems to work fine on MacOS and probably most text editors on Linux (i.e. EOL style doesn't matter, text is displayed properly). Also works for all types of IDE enabled editors on any platform. Actually the only commonly used text editor that has a problem with EOLs is Notepad. However agree that more tests/feedback is needed.

Leaving mixed line endings when different people edit the same file offline is a lot worse imho.

Last edited 12 years ago by azaozz (previous) (diff)

#21 @mbijon
12 years ago

I like the approach of doing this in WP, instead of with fopen. And, I know that convention is to never change a user's file, but for mixed line-endings I think we have to.

In general I've always heard that \n is the "universal" line ending. There's already a long-standing decision to use \n newlines in wpautop():

$pee = str_replace(array("\r\n", "\r"), "\n", $pee); // cross-platform newlines

Admittedly that's applied to content & not files. Though files & content are rendered by the browser, and the above works globally there. Meanwhile all modern editors do newline detection & replacement (and have settings for it), and I'm really against adding newline options to WP.

As for not being able to replicate this bug, it's specific to the combination of browser and server (possibly even some PHP config settings like language or Unicode). I've never seen it on WordPress, but have on other systems (one running on an enterprise Java/Solaris stack). It could be from Windows-based IE & Opera doing a transformation from \n to \r\n internally, http://stackoverflow.com/questions/1155678/javascript-string-newline-character/1156388#1156388

#22 @chriscct7
9 years ago

@nacin Did you want to close this as wontfix then?

#23 @iseulde
9 years ago

  • Component changed from Editor to Themes

Not sure where this belongs, but not here.

#24 @qui_gon_jim
9 years ago

I too have noticed this behaviour. Sometimes, after a website has gone live, our team may make quick small changes to the CSS using the inbuilt theme editor, rather than downloading the file and editing it.

/wp-admin/theme-editor.php?file=style.css

When I next download the CSS file, and open it in Notepad or Sublime2, every line has an extra \n after it which makes the file larger and harder to read. I have tested the editing in Windows Firefox 37.0.1 and Windows IE 11 and both of these cause the issue.

Sometimes I just disable the file editor to stop others using it!

define('DISALLOW_FILE_EDIT', true);

#25 @chriscct7
9 years ago

  • Component changed from Themes to Editor

This should belong in editor. The issue is the newlines used after editing a file in the editor. This isn't a theme component issue because it also affects the plugin editor.

#26 follow-up: @dd32
9 years ago

  • Component changed from Editor to Administration

Moving to generic Admin. (Editor = The posts editor, not Plugin/Theme Editors)

#27 in reply to: ↑ 26 @chriscct7
9 years ago

Replying to dd32:

Moving to generic Admin. (Editor = The posts editor, not Plugin/Theme Editors)

Ah my apologies. I didn't realize that.

#28 @SergeyBiryukov
21 months ago

#52769 was marked as a duplicate.

#29 follow-up: @desrosj
20 months ago

  • Resolution set to wontfix
  • Status changed from reopened to closed

I'm going to close this one out as a wontfix. There has been no meaningful activity in 10 years.

It's also highly likely that this problem was fixed somewhere along the way. In WordPress 4.9, CodeMirror was merged into Core to improve the way plugin and theme files were edited in the admin area.

#30 in reply to: ↑ 29 @SergeyBiryukov
20 months ago

  • Milestone set to Future Release
  • Resolution wontfix deleted
  • Status changed from closed to reopened

Replying to desrosj:

It's also highly likely that this problem was fixed somewhere along the way.

Based on #52769 (which I closed as a duplicate of this one), the issue is still relevant, so I would like to reopen :)

I think detecting line endings of the file being edited and preserving that format, as suggested in comment:15, would be the way to go here.

Note: See TracTickets for help on using tickets.