PerfectSlayer / scrollupfolder

A firefox addon wich goes up a folder of a website.
https://addons.mozilla.org/fr/firefox/addon/scroll-up-folder/
Other
21 stars 9 forks source link

Add parse domain feature #46

Closed debuggings closed 6 years ago

debuggings commented 6 years ago

Eg. From https://addons.mozilla.org/en-US/firefox/ you might want to go to https://mozilla.org/

PerfectSlayer commented 6 years ago

Hi @debuggings,

Thanks for the PR! It's a feature I forgot to implement when I port to WebExtensions. I have no time right now to test and merge it but I will in a near future! 😉

One last thing, and I don't have the answer, it's more to discuss about it but I previously add the "www." when missing. Should we keep it? Check the source code and the expected result to see more details about this case.

debuggings commented 6 years ago

Hi,

I think there is no need to keep the "www.", because it is another branch of the origin URL. Just like https://addons.mozilla.org/en-US/firefox/themes and "https://addons.mozilla.org/en-US/firefox/extensions.

The only difference is that it is very common that many sites have set their main domain started with "www". But it is also very common that URLs without "www" should automatically jump to the URL with "www", like facebook. More over, there are sites, like twitter, whoes main domains don't have the "www" part, so we should not add the "www" too.

PerfectSlayer commented 6 years ago

Ok, you convince me! I'll build the a version the next week and prepare changelog and usage wiki pages update. Thanks!

PerfectSlayer commented 6 years ago

Hi just found and fix an issue about the settings loading. It prevents your feature to be enabled by default.

The settings which was loaded does not contains the key parseDomain so the applySettings() function disable it:

/**
 * Apply the settings.
 * @param settings The settings to apply.
 */
function applySettings(settings) {
    // Update current settings
    displayUrlbarIcon = settings.displayUrlbarIcon;
    enableShortcuts = settings.enableShortcuts;
    parseAnchor = settings.parseAnchor;
    parseDomain = settings.parseDomain; // NOT DEFINED, so false
    parseGetVariables = settings.parseGetVariables;
    // Clear URLs cache
    urlCache = {};
    // Update urlbar icon
    browser.tabs.query({}).then(tabs => {
        for (let tab of tabs) {
            updateUrlbarIcon(tab);
        }
    });
}

I just changed the "Update current settings" block by checking if the value are set and equals to false:

    // Update current settings
    displayUrlbarIcon = !(settings.displayUrlbarIcon === false);
    enableShortcuts = !(settings.enableShortcuts === false);
    parseAnchor = !(settings.parseAnchor === false);
    parseDomain = !(settings.parseDomain === false);
    parseGetVariables = !(settings.parseGetVariables === false);

So it should work well with this fix: c02c234a3f8f7493adb5d2161bbed54c3aef16d7 👍