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

Go to default root host of domain #47

Closed
Gitoffthelawn opened this issue Mar 24, 2018 · 18 comments
Closed

Go to default root host of domain #47

Gitoffthelawn opened this issue Mar 24, 2018 · 18 comments

Comments

@Gitoffthelawn
Copy link
Contributor

Gitoffthelawn commented Mar 24, 2018

Several times recently, I've needed to go to the default "root" host of a domain.

So let's say I'm on gitoffthelawn.github.com. I then want to go to www.github.com.

Can you add it to the top of the list in the scrollupfolder dropdown?

For example:

www.github.com
gitoffthelawn.github.com
gitoffthelawn.github.com/folder
gitoffthelawn.github.com/folder/page.html

@PerfectSlayer
Copy link
Owner

Hi @Gitoffthelawn ffthelawn

If fact, it is the expected behavior. As the issue #10 points out, the www sub-domain is not a top domain. In fact, your example demonstrates it: www.github.com is not an upper domain of gitoffthelawn.github.com.

Usually, when the www is the top domain, going to domain.com redirects to www.domain.com so it should work as you expect. Otherwise, it seems good to me to go to domain.com and not www.domain.com if they are different.

What do you think about it?

@Gitoffthelawn
Copy link
Contributor Author

Hi!

I agree. Well, almost completely. Sometimes domain.com won't work, and you need to go to www.domain.com. I suppose both could be listed, with domain.com on top.

I think there may be a bug though. If you go to http://forum.freecommander.com/, and then press the extension's button, neither freecommander.com nor www.freecommander.com is listed as an option.

@PerfectSlayer
Copy link
Owner

Hum, if we add the www.domain.com, it should be on top:

  1. www.domain.com
  2. domain.com
  3. forum.domain.com
  4. forum.domain.com/members/

Because the logical parent of forum.domain.com is domain.com and www.domain.com should be added as first default element if not already present in the list.

@Gitoffthelawn
Copy link
Contributor Author

Ya, you're probably right... although it would look prettier in the other order. :)

Of course, your logic is spot on.

BTW, could you reproduce the bug on freecommander.com mentioned above (or is it just functionality not implemented yet)?

@PerfectSlayer
Copy link
Owner

Sorry, I did not catch the freecommander.com issue.

I just setup an environment and give it a try. It looks like it work for me:
2018-03-31-093801_1366x735_scrot

Do you have the same result?

@Gitoffthelawn
Copy link
Contributor Author

Weird. I get a completely different result. Mine doesn't list http://freecommander.com.

Tested on Win7 with FF60b8.

@PerfectSlayer
Copy link
Owner

Yes weird… I also run Firefox 60.0b8 (but on ChromeOS using crouton to run Debian. That's an other story!)

Maybe we could try to find what differs from my setup:

  • Do you modified the add-on?
  • What are the exact URL your test?
  • What are your add-on settings (they are all checked by default)?

@Gitoffthelawn
Copy link
Contributor Author

  1. Nope!
  2. http://forum.freecommander.com/
  3. All checked.

@Gitoffthelawn
Copy link
Contributor Author

Just tried it in a new FF profile. Same result. 8-)

@PerfectSlayer
Copy link
Owner

When you said "same result", it means same result as me (working) or same result as your default profile (not working)? 😥

@PerfectSlayer
Copy link
Owner

💡 ! Could you try to:

  1. Disable Parse domain option,
  2. Reboot Firefox,
  3. Enable Parse domain option,
  4. Test http://forum.freecommander.com again.

It might be a default setting value which is not applied.

@Gitoffthelawn
Copy link
Contributor Author

When you said "same result", it means same result as me (working) or same result as your default profile (not working)?

LOL. That was ambiguous! Same result as my default profile (not working).

Could you try to...

I don't see a Parse domain option in v6.01. I do see a Parse anchor option. I tuned that off. Restarted FF. Turned it on. Restarted FF. Tested. Still getting same result as I was getting before (not working).

@PerfectSlayer
Copy link
Owner

Okay, I see the issue here… I did not release the 6.1 version with the parsing domain option!
I just tried it and I found some bugs remains:

  • Error while parsing «two dots domains» (ex .co.uk),
  • Error while parsing IPv4 address.

I would really like to code unit tests before releasing this kind of feature but I don't know how to make it for WebExtensions...

@Gitoffthelawn
Copy link
Contributor Author

I had a hunch we were using different versions of the extension! :)

@PerfectSlayer
Copy link
Owner

Okay so I find a way to write some tests! 🎉
I create another JS file with path computation which should not depends of browser or WebExtensions API. I load it in extension manifest and require it npm test script.
It should work even if it looks dirty right now. But I now can write some test and the expected results!
So we can easily discuss about the behavior to code! 👍

I will try to find some time during the weekend to test and finish the domain parsing implementation. Can't promise anything 😅

@Gitoffthelawn
Copy link
Contributor Author

Gitoffthelawn commented Apr 1, 2018

That's great! Woohoo! :)

@PerfectSlayer
Copy link
Owner

Here it is! I just found a network connection to push it to GitHub: fafe7db
You could check the expected behavior reading the test cases 😉

I now need to update documentation and make a new release.
I also would like to ship the PR #44 but I don't know when it will be ready.

@PerfectSlayer
Copy link
Owner

As the version was approved on AMO, I close the issue!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants