Skip to content
Snippets Groups Projects

Update options.xul

Merged Ghost User requested to merge patch-1 into master

Change address to HTTPS

Merge request reports

Loading
Loading

Activity

Filter activity
  • Approvals
  • Assignees & reviewers
  • Comments (from bots)
  • Comments (from users)
  • Commits & branches
  • Edits
  • Labels
  • Lock status
  • Mentions
  • Merge request status
  • Tracking
  • Nice touch, thanks a lot for contributing!

    Edited by Thomas Rientjes
  • Author Contributor

    Created by: Gitoffthelawn

    Is there a reference that shows the benefit of using https for that internal pseudo-url?

  • @Gitoffthelawn I don't think there's a real practical benefit here. I tested it, and it did not seem to cause any problems, so I saw it as a nice symbolic change. It's mainly a statement.

    If there are any downsides I'm not aware of, please let me know!

    Edited by Thomas Rientjes
  • Author Contributor

    Created by: Gitoffthelawn

    @Synzvato I honestly don't know. I've written code that includes that pseudo-url thousands of times (and have read many other people's code that includes it), and I've never seen https used. But just because I have never seen something doesn't mean it's not a good idea!

    So I don't know if there are any advantages or disadvantages. The disadvantage I can think of is if it confuses the browser in some way or fails to serve it's purpose. This assumes that using https there is non-standard, which is my hypothesis based on personal experience. I'm very happy to learn something new, however.

    Another potential problem could be forcing an SSL certificate verification for a URL that does not have a certificate.

    If it makes sense to use https for that pseudo-URL, I think it's a great idea to use it (and I'll change my code everywhere!). But I think it's best to understand the rationale and consequences.

    @ekaris Why did you make this change? Do you have any supporting documentation, blogs, knowledgebases, etc. that can help understand this change?

  • So I don't know if there are any advantages or disadvantages. The disadvantage I can think of is if it confuses the browser in some way or fails to serve it's purpose.

    That was my first concern as well, so I built a copy of the add-on with the updated options.xul. No warnings were raised when loading the add-on and the settings page displayed just fine.

    This assumes that using https there is non-standard [...].

    This is most certainly true, using https there is definitely non-standard. It's purely a namespace identifier, not a resource locator. It serves the following two purposes:

    1. Disambiguate between two elements that happen to share the same name.
    2. Group elements relating to a common idea together.

    Another potential problem could be [...] verification for a URL that does not have a certificate.

    Well, it's quite important to note that it's never dereferenced to fetch a resource from the web. Still, the domain in question, www.mozilla.org, has a valid certificate and accepts HTTPS connections.

    I think it's best to understand the rationale and consequences.

    I think that's a valid point. While this change doesn't seem to break the preferences page in any way, messing with namespaces might be a bad way of promoting secure connections.

    Edited by Thomas Rientjes
  • Author Contributor

    I did the change because in my fast review I though the link would be fetched, I checked if it was working in HTTPS and edited the file, nothing more I'm afraid.

  • Since it appears to be confusing people, doesn't really look like a statement of any sort, and since it's hard to predict what exact side-effects this could have, I'll revert the changes.

    Thanks for weighing in @Gitoffthelawn and @ekaris, appreciated!

Please register or sign in to reply
Loading