Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

FIX: removes commented original output which was breaking display in FF3... #150

Closed
wants to merge 2 commits into from

Conversation

edavey
Copy link

@edavey edavey commented Apr 2, 2013

This commit removes the commented original output of the error (right at the end of the body element) as it was breaking the display in Firefox 3 which I (and possibly many others?) still use as my primary development browser.

The tests all pass and the (excellent) tool still seems to work so I'm assuming that this normall hidden output is safe to remove?

thks

Ed

firefox_3_6

@haileys
Copy link
Collaborator

haileys commented Apr 2, 2013

Ah, so this is what causes that problem! I've had this reported before but I was not able to repro it.

The commented exception information is actually there so developers encountering an exception when using curl can still see what went wrong. I'd like to keep this feature.

Can you find a way to fix this on FF3 without removing the feature?

@edavey
Copy link
Author

edavey commented Apr 2, 2013

Ah, OK.

What if I stick the debug in an wrapper element and then set it to "display:none;" or use some other styling to make it "invisible" normally?

I know introducing wrapper elements is obnoxious but might be a pragmatic solution in this case?

thks

Ed

On 2 Apr 2013, at 12:40, Charlie Somerville wrote:

Ah, so this is what causes this problem! I've had this reported before but I was not able to repro it.

The commented exception information is actually there so developers encountering an exception when using curl can still see what went wrong. I'd like to keep this feature.

Can you find a way to fix this on FF3 without removing the feature?


Reply to this email directly or view it on GitHub.

@edavey
Copy link
Author

edavey commented Apr 2, 2013

Hi Charlie,

I've had another look at this but I'm having trouble getting the tool to output html over curl.

With:

  • removed commented call to <%== render('text') %> in main.erb
  • curl header set to text/html (curl -H "Content-Type: text/html" http://lvh.me:3000/error)

the app returns text.erb

Could you tell me how I can reproduce the problem that the <%== render('text') %> in main.erb is there to fix. Presumably some curl scenario I haven't considered…

thks

Ed

On 2 Apr 2013, at 12:40, Charlie Somerville wrote:

Ah, so this is what causes this problem! I've had this reported before but I was not able to repro it.

The commented exception information is actually there so developers encountering an exception when using curl can still see what went wrong. I'd like to keep this feature.

Can you find a way to fix this on FF3 without removing the feature?


Reply to this email directly or view it on GitHub.

@edavey
Copy link
Author

edavey commented Apr 2, 2013

better errors

@rstacruz
Copy link
Collaborator

rstacruz commented Apr 2, 2013

I'm guessing this might be a good way to do it:

<pre style="display: none">
<%= render('text') %>
</pre>

@rstacruz
Copy link
Collaborator

rstacruz commented Apr 2, 2013

Hm, actually the comment isn't there for curl, it's just there in case someone needs it. The wget/curl handling is done elsewhere.

@rstacruz
Copy link
Collaborator

rstacruz commented Apr 2, 2013

Try curl -H "Accept: text/html" http://lvh.me:3000/error :)

…html template just in case it is useful into a element which can be hidden. This means that the output is no longer messed up in Firefox 3.
@edavey
Copy link
Author

edavey commented Apr 3, 2013

Rico, thanks for clarifiying the purpose of the plain text output -- and for the curl tip! -- many thanks! I've included a test so that in future people know why the text is being rendered there.

@rstacruz
Copy link
Collaborator

rstacruz commented Apr 3, 2013

Just a bit of insight: i'm guessing this breaks FF3 because of hyphens. Technically, <!-- --- --> is not legal HTML.

From the W3C specs on HTML comments:

"White space is not permitted between the markup declaration open delimiter("<!") and the comment open delimiter ("--"), but is permitted between the comment close delimiter ("--") and the markup declaration close delimiter (">"). A common error is to include a string of hyphens ("---") within a comment. Authors should avoid putting two or more adjacent hyphens inside comments."

@haileys
Copy link
Collaborator

haileys commented Apr 4, 2013

In fact, let's nuke this feature completely.

I just hit a problem where I had an error thrown in an ERB template near some HTML comments. We don't escape the plaintext info presented in HTML comments because it's supposed to be readable as plain text. However this causes problems when the source code that's displayed contains -->.

@haileys haileys closed this in 62f72c8 Apr 4, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants