Opened 6 years ago

Closed 19 months ago

#12218 closed defect (fixed)

toolbar_button.js should do more null checks

Reported by: cypherpunks Owned by: zyan
Priority: Low Milestone:
Component: HTTPS Everywhere/EFF-HTTPS Everywhere Version:
Severity: Normal Keywords: rulesets
Cc: jrw32982@…, brade, mcs, gk Actual Points:
Parent ID: Points:
Reviewer: Sponsor:

Description

I have installed it on WinXP, Win7, Linux(Ubuntu) in Firefox 29.0.1. On XP, I see a long list of rules in the preferences screen. On Win7 and Linux, the rule list is empty. I have tried reinstalling and restarting, but to no avail. I can't tell if it's actually doing anything or not without any rulesets...

Child Tickets

Attachments (1)

0001-Bug-12218-toolbar_button.js-should-do-more-null-chec.patch (2.7 KB) - added by mcs 5 years ago.
a possible fix

Download all attachments as: .zip

Change History (13)

comment:1 Changed 6 years ago by gk

Component: - Select a componentEFF-HTTPS Everywhere
Owner: set to zyan

comment:2 Changed 6 years ago by zyan

Status: newneeds_information

I think we fixed this in 3.5.1.

You can test if rewrites are happening by going to a site like http://apple.com (will be HTTPS if you have HTTPS everywhere installed and running and haven't disabled the apple rule).

comment:3 Changed 6 years ago by matiash

I'm having this same issue in Firefox 30 and 31, Windows 7 64-bit. Tried both latest-stable, 3.5.3, and 4.0-development-17. The list is empty, and apple.com does not redirect me to https.

This used to work in the past, I'm not sure exactly when it broke. Anything I can do to help debug it?

Last edited 6 years ago by matiash (previous) (diff)

comment:4 Changed 5 years ago by zyan

!!

Does this work in Firefox 29?

Can you go to your Firefox profile folder (see https://support.mozilla.org/en-US/kb/profiles-where-firefox-stores-user-data#w_how-do-i-find-my-profile), and see if ./extensions/https-everywhere@…/defaults/rulesets.sqlite exists?

Also you can try going to about:config and setting extensions.https_everywhere.LogLevel to 2, restarting Firefox, and saving the console output.

comment:5 in reply to:  4 Changed 5 years ago by matiash

The sqlite file is indeed there. But from the log, I think I found the cause. It said:

TypeError: toolbarbutton is null toolbar_button.js:133
TypeError: popup is null toolbar_button.js:234
TypeError: popup is null toolbar_button.js:234

I had customized the Firefox toolbar to remove the HTTPS Everywhere icon. When I added it again, it started working.

From my side the issue is resolved, but if you need anything else I'd be glad to help.

Last edited 5 years ago by matiash (previous) (diff)

comment:6 Changed 5 years ago by jsha

Priority: criticalminor
Status: needs_informationneeds_revision
Summary: No rules show up after installing HTTPS Everywheretoolbar_button.js should do more null checks

I tried to reproduce this on Firefox 31.0. I installed HTTPS Everywhere, restarted, customized the toolbar to remove the HTTPS Everywhere icon, and restarted. I was still able to visit e.g. httP://www.openssl.org, and get redirected by the extension to HTTPS.

I'm not sure what's going on, but I do notice a few places in toolbar_button.js that could you some protective null checks. I'm going to downgrade priority and rename.

comment:7 Changed 5 years ago by mcs

Cc: brade mcs gk added

comment:8 Changed 5 years ago by mcs

Status: needs_revisionneeds_review

Kathy Brade and I saw various errors on the Browser Console when testing Tor Browser 4.5a5 that have their origin in HTTPS Everywhere (all were related to no toolbar item being present). The patch that I just attached fixes all of them for us, including the one described in #13550.

comment:9 Changed 5 years ago by zyan

Hey mcs, the patch looks good but please use curly braces around all blocks.

Also, if you don't mind submitting the patch as a Github pull request, that would be preferred. https://github.com/efforg/https-everywhere/pulls

BTW, I am no longer officially working for EFF/Tor, so I generally don't see email notifications for tickets owned by this account. The active maintainers mostly use Github, I think.

comment:10 in reply to:  9 Changed 5 years ago by mcs

Replying to zyan:

Hey mcs, the patch looks good but please use curly braces around all blocks.

Fixed.

Also, if you don't mind submitting the patch as a Github pull request, that would be preferred. https://github.com/efforg/https-everywhere/pulls

Done. Thanks for pointing me in the right direction for patch submission.

comment:11 Changed 2 years ago by teor

Severity: Normal

Set all open tickets without a severity to "Normal"

comment:12 Changed 19 months ago by cypherpunks

Resolution: fixed
Status: needs_reviewclosed

Looks like it's been fixed on the deprecated XUL/XPCOM, we're on the WebExtension now anyway.

Note: See TracTickets for help on using tickets.