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

Frames link is broken; the right frame is empty #744

Closed
DavidEGrayson opened this issue Mar 1, 2014 · 3 comments
Closed

Frames link is broken; the right frame is empty #744

DavidEGrayson opened this issue Mar 1, 2014 · 3 comments

Comments

@DavidEGrayson
Copy link
Contributor

I would like to report a bug with the "Frames" HTML link generated by YARD. It does not work properly.

I am using:

  • The official YARD gem, version 0.8.7.3
  • jruby 1.7.10 (1.9.3p392) 2014-01-09 c4ecd6b on Java HotSpot(TM) 64-Bit Server VM 1.7.0_45-b18 [Windows 8-amd64]

To reproduce the issue, I made a simple Ruby file called test.rb with a single class with a single empty method. I think the exact contents of that file are not important. I ran bundle exec yard from a Git Bash shell.

YARD generates a file called _index.html which has the following link:

<a href="frames.html#!file%3A///C%3A/Users/David/Documents/scraps/test_yard/doc/_index.html" target="_top">frames</a>

First of all, is that link what you would expect? It seems to me that you shouldn't need to put the full, absolute path in the URL. But that is not the problem.

The problem is that when I click on the link and go to that URL, the right frame (the one that is supposed to have all the content) is broken because it is trying to load an invalid URL. Instead of seeing content, I see a little message from Google Chrome that says "The file or directory could not be found.". If I press Ctrl+Shift+I in Chrome then I can inspect the elements in the DOM, and I see that the problematic frame element (which is generated by javascript), is:

<frame name="main" src="/C%3A/Users/David/Documents/scraps/test_yard/doc/_index.html">

Here is a screenshot:

image

The problem here is that the javascript code in frames.html has purposely removed the protocol from the URL, so the web browser will misinterpret it. In the example above, I was just using local files on my computer, but the same problem happens for files that I have uploaded to the web and am viewing over http.

I would like to fix this and make a pull request for it. Perhaps @lsegal could tell me how it should be fixed? I see two options:

  1. Just change the javascript in frames.html and fix it to not remove the protocol. (Why was the protocol being removed in the first place?)
  2. Change the javascript in frames.html, but also change the code that generates the link so that it uses relative URLs. This will make the URLs shorter, which is nice.

Whatever we do, we should be careful not to break existing links and bookmarks that people might have to YARD documentation.

@lsegal
Copy link
Owner

lsegal commented Mar 1, 2014

The bug is that we're generating the full URL in the first place, so the fix should be (2). 98f538a is the change that removes the protocol-- it's necessary to avoid XSS attacks. The attack vector is that someone can put any manufactured URL in the anchor, give someone the full URL and potentially have them click unsafe links.

We can avoid this if we just use rel links, but I'm pretty sure the frames link is just using window.location, which is why it ends up being an absolute URL:

https://github.com/lsegal/yard/blob/master/templates/default/fulldoc/html/js/app.js#L89

and

https://github.com/lsegal/yard/blob/master/templates/default/layout/html/script_setup.erb

I think instead of window.location.href there you could maybe use <%= url_for(object, nil, false) %> but you might have to experiment.

@DavidEGrayson
Copy link
Contributor Author

Thanks for the many hints! I am working on this now. Your suggestion works for most of the pages, but not for pages that are generated from extra markdown files (e.g. file.README.html). For such pages, the object always seems to be #<yardoc root> and the url that gets generated for it is top-level-namespec.html. I will look for another variable that helps tell what HTML file we are currently generating.

@lsegal
Copy link
Owner

lsegal commented Mar 2, 2014

Check the layout template for that object

DavidEGrayson added a commit to DavidEGrayson/yard that referenced this issue Mar 2, 2014
@lsegal lsegal closed this as completed in 3e84b2d Mar 2, 2014
lsegal added a commit that referenced this issue Mar 22, 2014
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

No branches or pull requests

2 participants