Opened 9 years ago

Closed 9 years ago

#5352 closed enhancement (fixed)

option to stop telling me firefox is going to start

Reported by: phobos Owned by: sirop
Priority: Medium Milestone:
Component: Archived/Vidalia Version: Vidalia: 0.3.1-alpha
Severity: Keywords: plugin, tbb
Cc: Actual Points:
Parent ID: Points:
Reviewer: Sponsor:

Description

With vidalia 3.1-alpha, there is a new 'info message' or pop-up that occurs after tor bootstraps, but firefox hasn't started. there should be an option to disable/enable it.

Child Tickets

Attachments (1)

download.php?i=ChmFhCTP (4.8 KB) - added by rransom 9 years ago.

Download all attachments as: .zip

Change History (18)

comment:1 Changed 9 years ago by chiiph

Keywords: plugin tbb added

The message says that firefox will start soon, so it wouldn't be necessary if we had firefox already running.

That said, a checkbox to disable it in the tbb plugin configuration won't hurt.

comment:2 Changed 9 years ago by sirop

Status: newneeds_information

Does this mean we can load the tbb plugin without restarting the firefox?

comment:3 in reply to:  2 ; Changed 9 years ago by sirop

Owner: changed from chiiph to sirop
Status: needs_informationassigned

Replying to sirop:

Does this mean we can load the tbb plugin without restarting the firefox?

O.k. To reproduce the bug:
./start-tor-browser
wait until Firefox with the tbb plugin is loaded
stop TOR in the Vidalia menu
restart TOR in the Vidalia menu
see the message "firefox will start soon" although Firefox is already running

My proposal: either have an option to disable this info message or have a different info message saying something like "Firefox is already running. If the Tor network is reestablished, feel free to surf".

comment:4 in reply to:  3 Changed 9 years ago by sirop

Now one can uncheck "Show tray notifications for Firefox".
However, if unchecked or checked, this new state is valid only after the Tor Browser Bundle is restarted.
If it is the very functionality needed, I can upload my tbb.js to gitorous.

comment:5 Changed 9 years ago by chiiph

The whole point of this is to find a solution, so if you've got a possible one, please show us.

You should create a branch named something like bug5352_notification with this fix, so I can check it out and merge if it's ok. Mark the ticket as needs_review when you propose a patch/branch with a fix, and I'll either close it and merge or mark it as needs_revision with some comments on what you should change. When those changes are done, you mark it as needs_review again pointing to the place where the changes are, and so on.

comment:6 in reply to:  5 Changed 9 years ago by sirop

I cannot upload anything either to github or gitorious,
always get "fatal: The remote end hung up unexpectedly" error. Don't know why.

That's why git diff output at pastebin: http://pastebin.com/ChmFhCTP 

for the time being.

comment:7 in reply to:  5 Changed 9 years ago by sirop

Status: assignedneeds_review

Changed 9 years ago by rransom

Attachment: download.php?i=ChmFhCTP added

comment:8 Changed 9 years ago by chiiph

Status: needs_reviewneeds_revision

Overall looks good. A couple of notes:

  • If I start TBB with DontShowTrayBox=false, then change it to true and I restart tor, I will still see the notification.
  • You have changed the way the settings behave, which is another issue, not this one.
  • It's Tor, not TOR.
  • There are a couple of bad indentations (see the first if in that patch, for example).

comment:9 in reply to:  8 ; Changed 9 years ago by sirop

Replying to chiiph:

Overall looks good. A couple of notes:

  • If I start TBB with DontShowTrayBox=false, then change it to true and I restart tor, I will still see the notification.

I cannot reproduce this error. Maybe, you've forgotten to Save under Plugins --> Browser Bundle Settings.

  • You have changed the way the settings behave, which is another issue, not this one.

Yes, my fault. Had to do it as firefox did not start otherwise. Will try to make up a ticket for
launchBrowserFromDirectory function these days.

  • It's Tor, not TOR.

Will be corrected.

  • There are a couple of bad indentations (see the first if in that patch, for example).

Yes, I looked it up in https://gitweb.torproject.org/vidalia.git/blob/HEAD:/HACKING
Will be changed.

comment:10 in reply to:  9 ; Changed 9 years ago by chiiph

Replying to sirop:

Replying to chiiph:

Overall looks good. A couple of notes:

  • If I start TBB with DontShowTrayBox=false, then change it to true and I restart tor, I will still see the notification.

I cannot reproduce this error. Maybe, you've forgotten to Save under Plugins --> Browser Bundle Settings.

With "restart tor" I meant: click the "Stop tor" button, and then click the "Start tor" one.

Another way of putting this is: you don't check what the option is set to inside showWaitDialog().

comment:11 in reply to:  10 Changed 9 years ago by sirop

Status: needs_revisionneeds_review

Replying to chiiph:
Bundle Settings.

With "restart tor" I meant: click the "Stop tor" button, and then click the "Start tor" one.

Another way of putting this is: you don't check what the option is set to inside showWaitDialog().

I changed showWaitDialog() as you propose.
https://github.com/sirop/vidalia-plugins/commit/a265d669704d3052da4ff36bac4e5ec63ea7e7ae

launchBrowserFromDirectory() was also changed, although "it is a different issue".
I think, I formulated https://trac.torproject.org/projects/tor/ticket/5364 not clear enough,
but I'll try to correct this until 5 p.m. German time to-day.

comment:12 Changed 9 years ago by chiiph

Status: needs_reviewneeds_revision

I still see bad indentation, too much empty lines.
There are config changes that don't belong here.

Also, the tbb plugin was updated, could you rebase your work for this on the last master?

comment:13 Changed 9 years ago by sirop

Status: needs_revisionneeds_review

https://github.com/sirop/vidalia-plugins/commits/bug5352_notifications

Based on the last master.
Hope, my indentation is no longer bad ;).

comment:14 Changed 9 years ago by chiiph

Status: needs_reviewneeds_revision

Three things:

  • Yes, bad indentation is still there. For the future, check the commit in github before posting it here, you'll see what I'm talking about (in case everything looks ok in your editor).
  • Why did you commented out this if(QDir(browserDirectory).isRelative()) ?
  • You should remove vdebugs like: vdebug(browserDirectory + " line 196, sirop");

comment:15 in reply to:  14 Changed 9 years ago by sirop

Replying to chiiph:

Three things:

  • Yes, bad indentation is still there. For the future, check the commit in github before posting it here, you'll see what I'm talking about (in case everything looks ok in your editor).
  • Why did you commented out this if(QDir(browserDirectory).isRelative()) ?
  • You should remove vdebugs like: vdebug(browserDirectory + " line 196, sirop");

One more try:
https://github.com/sirop/vidalia-plugins/commit/bea98c70ab5a130e7d3cad308dd37094ee879294

Yes, my editor showed something different before. :)

comment:16 Changed 9 years ago by sirop

Status: needs_revisionneeds_review

comment:17 Changed 9 years ago by chiiph

Resolution: fixed
Status: needs_reviewclosed

Looks good, merging.

Thanks!

Note: See TracTickets for help on using tickets.