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

Add view-source protocol #4181

Closed
jdm opened this issue Dec 2, 2014 · 14 comments
Closed

Add view-source protocol #4181

jdm opened this issue Dec 2, 2014 · 14 comments

Comments

@jdm
Copy link
Member

@jdm jdm commented Dec 2, 2014

Make a handler that delegates to the HTTP loader then sets the Content-Type header to text/plain!

@ajnirp
Copy link
Contributor

@ajnirp ajnirp commented Dec 3, 2014

If this isn't taken already, can I work on this?

@jdm
Copy link
Member Author

@jdm jdm commented Dec 3, 2014

Sure!

@jdm jdm added the C-assigned label Dec 3, 2014
@ajnirp
Copy link
Contributor

@ajnirp ajnirp commented Dec 5, 2014

I have some changes, but don't know how to test them. Do I just call ./mach run with something like view-source:<url>?

@jdm
Copy link
Member Author

@jdm jdm commented Dec 5, 2014

I think so? You may need to modify the URL that gets passed to the HTTP loader, but I'm not certain.

@SimonSapin
Copy link
Member

@SimonSapin SimonSapin commented Dec 5, 2014

view-source:http://example.com/ wouldn’t work well with the rust-url parser, since it assumes that URLs only have one scheme, and schemes can not contain :. So there are at least two options: either change the view-source: prefix to view-source+ (- and + are fine in URL schemes), or not parse as an URL the input of the command line argument or any URL bar UI until any view-source: prefix is removed at the string level.

@ajnirp
Copy link
Contributor

@ajnirp ajnirp commented Dec 23, 2014

I don't have the time to work on this now, sorry.

@Manishearth Manishearth removed the C-assigned label Dec 23, 2014
@doublec
Copy link
Contributor

@doublec doublec commented Mar 12, 2015

Is anyone working on this? If not, I'd be keen to try.

@jdm
Copy link
Member Author

@jdm jdm commented Mar 12, 2015

You're welcome to it!

@jdm jdm added the C-assigned label Mar 12, 2015
doublec added a commit to doublec/servo that referenced this issue Mar 13, 2015
This follows the recommendation from issue servo#4181. A handler
for 'view-source' delegates to the HTTP loader. In that
loader I check for view-source, adjust the URL to be the
URL to be viewed and modify the Content-Type header to be
text/plain.

This doesn't actually result in the source being
viewed as rendering text/plain is not yet implemented.
@doublec
Copy link
Contributor

@doublec doublec commented Mar 13, 2015

I've implemented the suggested approach of having a view-source handler, and modifying the content type to be text/plain if that is used. It's in doublec/servo@a788bcc.

The content doesn't render as text/plain though as, I assume from issue #3649, it is not treated specially for rendering. Requesting text/plain content formats it as HTML.

Should I pull request this as is? Or look at implementing text/plain beforehand?

@jdm
Copy link
Member Author

@jdm jdm commented Mar 13, 2015

If you're willing, implementing text/plain would be great! That check probably belongs in ScriptTask::load for now.

@doublec
Copy link
Contributor

@doublec doublec commented Mar 13, 2015

No worries, I'll get on to text/plain support!

@doublec
Copy link
Contributor

@doublec doublec commented Mar 13, 2015

Is there a spec or a pointer to the way Gecko does text/plain display? Taking your ScriptTask pointer I hacked in a simple "wrap the text in a pre element with '<' escaped" to get something working. This lets me at test that the view-source changes I made work. I've put that in doublec/servo@d953dad. The following works:

./mach run http://cd.pn/x.txt
./mach run view-source:http://tinyvid.tv/

For a better text/plain solution would it be better to build the document up manually in ScriptTask::load instead of parse_html? And is the "put it in a pre" approach ok, or is there a better way of building up the document for plain text?

@SimonSapin
Copy link
Member

@SimonSapin SimonSapin commented Mar 13, 2015

Dealing with text/plain is specified here: https://html.spec.whatwg.org/multipage/browsers.html#read-text

In particular, the HTML tokenizer has a PLAINTEXT mode where it just emits the input as text data and never switches mode again.

@doublec
Copy link
Contributor

@doublec doublec commented Mar 13, 2015

Aha, thanks, that clears things up.

doublec added a commit to doublec/servo that referenced this issue Mar 15, 2015
This follows the recommendation from issue servo#4181. A handler
for 'view-source' delegates to the HTTP loader. In that
loader I check for view-source, adjust the URL to be the
URL to be viewed and modify the Content-Type header to be
text/plain.

This doesn't actually result in the source being
viewed as rendering text/plain is not yet implemented.
doublec added a commit to doublec/servo that referenced this issue Mar 16, 2015
bors-servo pushed a commit that referenced this issue Mar 17, 2015
…xt, r=jdm

Implements view-source protocol by having a view-source handler, and modifying the content type to be text/plain if that is used. 

Implements text/plain handling. This allows view-source content to display as plain text.

Example usage:

    ./mach run http://cd.pn/x.txt
    ./mach run view-source:http://tinyvid.tv/

This fixes issue #4181. Issue #3649 includes "support text/plain" so this possibly fixes some of that issue as well.
bors-servo pushed a commit that referenced this issue Mar 17, 2015
…xt, r=jdm

Implements view-source protocol by having a view-source handler, and modifying the content type to be text/plain if that is used. 

Implements text/plain handling. This allows view-source content to display as plain text.

Example usage:

    ./mach run http://cd.pn/x.txt
    ./mach run view-source:http://tinyvid.tv/

This fixes issue #4181. Issue #3649 includes "support text/plain" so this possibly fixes some of that issue as well.
bors-servo pushed a commit that referenced this issue Mar 17, 2015
…xt, r=jdm

Implements view-source protocol by having a view-source handler, and modifying the content type to be text/plain if that is used. 

Implements text/plain handling. This allows view-source content to display as plain text.

Example usage:

    ./mach run http://cd.pn/x.txt
    ./mach run view-source:http://tinyvid.tv/

This fixes issue #4181. Issue #3649 includes "support text/plain" so this possibly fixes some of that issue as well.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
5 participants
You can’t perform that action at this time.