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

plaintext linkification #39

Merged
merged 2 commits into from
Jan 3, 2016
Merged

plaintext linkification #39

merged 2 commits into from
Jan 3, 2016

Conversation

the8472
Copy link
Contributor

@the8472 the8472 commented Dec 28, 2015

splitting out the linkification work from the url canonization since this pretty much works while the latter is a hairy mess.

@the8472
Copy link
Contributor Author

the8472 commented Dec 28, 2015

implements #27

@the8472 the8472 force-pushed the linkify branch 2 times, most recently from 0fc7bdd to 05e3b48 Compare December 28, 2015 23:25
@@ -0,0 +1,15 @@
<!doctype html>
<!-- open as resource://ipfs-firefox-addon-at-lidel-dot-org/data/test.html -->
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cool! It probably should be used in a dedicated test suite (test/test-linkify.js).

So far I did manual test and got weird results:

  1. Run via jpm -b vendor/firefox-45.0a2/firefox run
  2. I enabled at addon's about:addons screen:
    2016-01-01-182616_256x49_scrot
  3. Visited resource://ipfs-firefox-addon-at-lidel-dot-org/data/test.html
  4. Text links were not converted into <a> tags. The only change I was able to spot was the href of the "abs link", which changed from
    fs:/ to https://ipfs.io/

I did not dig into the underlying code: perhaps it is still work in progress?
Or did I found a bug? :-)

@the8472
Copy link
Contributor Author

the8472 commented Jan 2, 2016

Or did I found a bug? :-)

yeah, missed a change when rebasing. should work now.

I also added a test.

It may be a better idea to on/off entire listener based on checkbox state.

Eh, it bails out fairly early: https://github.com/lidel/ipfs-firefox-addon/pull/39/files#diff-d5184922475dad24022cebfa449bba33R166

lidel added a commit that referenced this pull request Jan 3, 2016
plaintext linkification + relative/native href fix
@lidel lidel merged commit 5fa2f05 into ipfs:master Jan 3, 2016
@lidel
Copy link
Member

lidel commented Jan 3, 2016

Thanks. Now works as advertised! 👌

(did some cleanup in a837132 & a74d4f7)

@lidel lidel added this to the v1.4.0 milestone Jan 3, 2016
lidel added a commit that referenced this pull request Feb 12, 2017
It was not possible to reuse code introduced in #39 (legacy SDK)

Requires performance tweaks, but for now closes #202
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.

None yet

2 participants