Re: [tor-bugs] #19273 [Applications/Tor Browser]: Write C++ patch to replace external applications helper dialog workaround

[Tor Bug Tracker & Wiki]

#19273: Write C++ patch to replace external applications helper dialog 
workaround
-+-
 Reporter:  gk   |  Owner:  mcs
 Type:  task | Status:  closed
 Priority:  High |  Milestone:
Component:  Applications/Tor Browser |Version:
 Severity:  Normal   | Resolution:  fixed
 Keywords:  TorBrowserTeam201607R, tbb-  |  Actual Points:
  usability, tbb-torbutton-conversion|
Parent ID:   | Points:
 Reviewer:   |Sponsor:
 |  SponsorU
-+-

Comment (by bugzilla):

 #18778, #18090, #18554 should be child tickets. Also regression tests are
 highly needed for this ticket.

--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [tor-bugs] #19273 [Applications/Tor Browser]: Write C++ patch to replace external applications helper dialog workaround

[Tor Bug Tracker & Wiki]

#19273: Write C++ patch to replace external applications helper dialog 
workaround
-+-
 Reporter:  gk   |  Owner:  mcs
 Type:  task | Status:  closed
 Priority:  High |  Milestone:
Component:  Applications/Tor Browser |Version:
 Severity:  Normal   | Resolution:  fixed
 Keywords:  TorBrowserTeam201607R, tbb-  |  Actual Points:
  usability, tbb-torbutton-conversion| Points:
Parent ID:   |Sponsor:
 Reviewer:   |  SponsorU
-+-

Comment (by brade):

 Replying to [comment:9 gk]:
 > Okay, finally. Looks good to me and I am happy to see my crazy
 workaround going. I made a fixup commit removing the remaining bits from
 my #9901 patch we don't need anymore
 (7dd710152955bd4145814c8bafed3b50dfacff75 on torbutton master). mcs' and
 brade's patches are commit 6d5dc7845579ca99d9c46d921876e477ba60935f on
 torbutton master and commit 4b7123f235465f26052ea0dfaeebb590a3f31e0f on
 tor-browser-45.2.0esr-6.5-1.

 Thanks for your review and for catching what we missed.

--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [tor-bugs] #19273 [Applications/Tor Browser]: Write C++ patch to replace external applications helper dialog workaround

[Tor Bug Tracker & Wiki]

#19273: Write C++ patch to replace external applications helper dialog 
workaround
-+-
 Reporter:  gk   |  Owner:  mcs
 Type:  task | Status:  closed
 Priority:  High |  Milestone:
Component:  Applications/Tor Browser |Version:
 Severity:  Normal   | Resolution:  fixed
 Keywords:  TorBrowserTeam201607R, tbb-  |  Actual Points:
  usability, tbb-torbutton-conversion| Points:
Parent ID:   |Sponsor:
 Reviewer:   |  SponsorU
-+-
Changes (by gk):

 * keywords:  TorBrowserTeam201607R, tbb-usability => TorBrowserTeam201607R,
 tbb-usability, tbb-torbutton-conversion


--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [tor-bugs] #19273 [Applications/Tor Browser]: Write C++ patch to replace external applications helper dialog workaround

[Tor Bug Tracker & Wiki]

#19273: Write C++ patch to replace external applications helper dialog 
workaround
-+-
 Reporter:  gk   |  Owner:  mcs
 Type:  task | Status:  closed
 Priority:  High |  Milestone:
Component:  Applications/Tor Browser |Version:
 Severity:  Normal   | Resolution:  fixed
 Keywords:  TorBrowserTeam201607R, tbb-  |  Actual Points:
  usability  | Points:
Parent ID:   |Sponsor:
 Reviewer:   |  SponsorU
-+-
Changes (by gk):

 * cc: bugzilla (added)


Comment:

 #18090 is a duplicate of this one.

--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [tor-bugs] #19273 [Applications/Tor Browser]: Write C++ patch to replace external applications helper dialog workaround

[Tor Bug Tracker & Wiki]

#19273: Write C++ patch to replace external applications helper dialog 
workaround
-+-
 Reporter:  gk   |  Owner:  mcs
 Type:  task | Status:  closed
 Priority:  High |  Milestone:
Component:  Applications/Tor Browser |Version:
 Severity:  Normal   | Resolution:  fixed
 Keywords:  TorBrowserTeam201607R, tbb-  |  Actual Points:
  usability  | Points:
Parent ID:   |Sponsor:
 Reviewer:   |  SponsorU
-+-
Changes (by gk):

 * keywords:  TorBrowserTeam201607R => TorBrowserTeam201607R, tbb-usability
 * status:  needs_review => closed
 * resolution:   => fixed


Comment:

 Okay, finally. Looks good to me and I am happy to see my crazy workaround
 going. I made a fixup commit removing the remaining bits from my #9901
 patch we don't need anymore (7dd710152955bd4145814c8bafed3b50dfacff75 on
 torbutton master). mcs' and brade's patches are commit
 6d5dc7845579ca99d9c46d921876e477ba60935f on torbutton master and commit
 4b7123f235465f26052ea0dfaeebb590a3f31e0f on tor-browser-45.2.0esr-6.5-1.

 This will be in the next alpha for broader testing. I expect this will
 help with usability things like #18090 as well.

--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [tor-bugs] #19273 [Applications/Tor Browser]: Write C++ patch to replace external applications helper dialog workaround

[Tor Bug Tracker & Wiki]

#19273: Write C++ patch to replace external applications helper dialog 
workaround
--+--
 Reporter:  gk|  Owner:  mcs
 Type:  task  | Status:  needs_review
 Priority:  High  |  Milestone:
Component:  Applications/Tor Browser  |Version:
 Severity:  Normal| Resolution:
 Keywords:  TorBrowserTeam201607R |  Actual Points:
Parent ID:| Points:
 Reviewer:|Sponsor:  SponsorU
--+--

Comment (by mcs):

 Replying to [comment:7 arthuredelstein]:
 > I have looked over the code and I think it looks good. (I am still
 building it and will report if I see any problems.)

 Thanks for looking at the code.

 > General question: do you think a fix for
 https://bugzilla.mozilla.org/show_bug.cgi?id=440892 would use this
 observer mechanism? Or could a proper check for "network.protocol-handler
 .warn-external" be simply built directly into the code? I'm wondering what
 the right approach would be for future upstreaming.

 I think a proper fix should just be built into the code (probably inside
 uriloader/exthandler/nsHandlerService.js). But given the uncertainty that
 was expressed by mikeperry in the following comment, Kathy and I thought
 it would be best to mimic the existing hooks that were in Torbutton
 (replacing the component overrides with an observer notification):
 https://trac.torproject.org/projects/tor/ticket/9901#comment:74

 Also, if we want to include a custom message it is easier to have our own
 separate dialog (although we did experiment with using a XUL overlay; see
 ticket:16623#comment:3).

--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [tor-bugs] #19273 [Applications/Tor Browser]: Write C++ patch to replace external applications helper dialog workaround

[Tor Bug Tracker & Wiki]

#19273: Write C++ patch to replace external applications helper dialog 
workaround
--+--
 Reporter:  gk|  Owner:  mcs
 Type:  task  | Status:  needs_review
 Priority:  High  |  Milestone:
Component:  Applications/Tor Browser  |Version:
 Severity:  Normal| Resolution:
 Keywords:  TorBrowserTeam201607R |  Actual Points:
Parent ID:| Points:
 Reviewer:|Sponsor:  SponsorU
--+--

Comment (by arthuredelstein):

 I have looked over the code and I think it looks good. (I am still
 building it and will report if I see any problems.)

 General question: do you think a fix for
 https://bugzilla.mozilla.org/show_bug.cgi?id=440892 would use this
 observer mechanism? Or could a proper check for "network.protocol-handler
 .warn-external" be simply built directly into the code? I'm wondering what
 the right approach would be for future upstreaming.

--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [tor-bugs] #19273 [Applications/Tor Browser]: Write C++ patch to replace external applications helper dialog workaround

[Tor Bug Tracker & Wiki]

#19273: Write C++ patch to replace external applications helper dialog 
workaround
--+--
 Reporter:  gk|  Owner:  mcs
 Type:  task  | Status:  needs_review
 Priority:  High  |  Milestone:
Component:  Applications/Tor Browser  |Version:
 Severity:  Normal| Resolution:
 Keywords:  TorBrowserTeam201606R |  Actual Points:
Parent ID:| Points:
 Reviewer:|Sponsor:  SponsorU
--+--

Comment (by mcs):

 Here is a revised Torbutton patch that follows the protocol I described in
 comment:4:
 
https://gitweb.torproject.org/user/brade/torbutton.git/commit/?h=bug19273-02=5e92e8c171ded5125aeeccfc2e6a4cfb974412ac

--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [tor-bugs] #19273 [Applications/Tor Browser]: Write C++ patch to replace external applications helper dialog workaround

[Tor Bug Tracker & Wiki]

#19273: Write C++ patch to replace external applications helper dialog 
workaround
--+--
 Reporter:  gk|  Owner:  mcs
 Type:  task  | Status:  needs_review
 Priority:  High  |  Milestone:
Component:  Applications/Tor Browser  |Version:
 Severity:  Normal| Resolution:
 Keywords:  TorBrowserTeam201606R |  Actual Points:
Parent ID:| Points:
 Reviewer:|Sponsor:  SponsorU
--+--

Comment (by mcs):

 Replying to [comment:3 cypherpunks]:
 > It looks like you will get a random result, if there are multiple
 observers that disagree. Presumably, there is only one right now, but what
 happens if this code is upstreamed?

 Good point. The observer service does not provide a way to avoid notifying
 all the observers, but we could specify that all modules that handle
 "external-app-requested" should not do anything if cancel has already been
 requested (subject.data == true). I think that will solve the problem.

--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [tor-bugs] #19273 [Applications/Tor Browser]: Write C++ patch to replace external applications helper dialog workaround

[Tor Bug Tracker & Wiki]

#19273: Write C++ patch to replace external applications helper dialog 
workaround
--+--
 Reporter:  gk|  Owner:  mcs
 Type:  task  | Status:  needs_review
 Priority:  High  |  Milestone:
Component:  Applications/Tor Browser  |Version:
 Severity:  Normal| Resolution:
 Keywords:  TorBrowserTeam201606R |  Actual Points:
Parent ID:| Points:
 Reviewer:|Sponsor:  SponsorU
--+--

Comment (by cypherpunks):

 It looks like you will get a random result, if there are multiple
 observers that disagree. Presumably, there is only one right now, but what
 happens if this code is upstreamed?

--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [tor-bugs] #19273 [Applications/Tor Browser]: Write C++ patch to replace external applications helper dialog workaround

[Tor Bug Tracker & Wiki]

#19273: Write C++ patch to replace external applications helper dialog 
workaround
--+--
 Reporter:  gk|  Owner:  mcs
 Type:  task  | Status:  needs_review
 Priority:  High  |  Milestone:
Component:  Applications/Tor Browser  |Version:
 Severity:  Normal| Resolution:
 Keywords:  TorBrowserTeam201606R |  Actual Points:
Parent ID:| Points:
 Reviewer:|Sponsor:  SponsorU
--+--
Changes (by mcs):

 * cc: arthuredelstein (added)
 * keywords:  TorBrowserTeam201606 => TorBrowserTeam201606R
 * status:  assigned => needs_review


Comment:

 Here is a tor-browser patch that generates "external-app-requested"
 observer service notifications that Torbutton can use to prompt and
 optionally cancel the launch of an external application:
 https://gitweb.torproject.org/user/brade/tor-
 browser.git/commit/?h=bug19273-01=0dff6c74126b5b52d4d7504644365af30985042b

 and here is a Torbutton patch that uses the new notification (and removes
 a lot of old JS code):
 
https://gitweb.torproject.org/user/brade/torbutton.git/commit/?h=bug19273-01=d9ce9236ab4e2d371426e9e25d16e804ed5b2332

 Please review.

--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [tor-bugs] #19273 [Applications/Tor Browser]: Write C++ patch to replace external applications helper dialog workaround

[Tor Bug Tracker & Wiki]

#19273: Write C++ patch to replace external applications helper dialog 
workaround
--+--
 Reporter:  gk|  Owner:  mcs
 Type:  task  | Status:  assigned
 Priority:  High  |  Milestone:
Component:  Applications/Tor Browser  |Version:
 Severity:  Normal| Resolution:
 Keywords:  TorBrowserTeam201606  |  Actual Points:
Parent ID:| Points:
 Reviewer:|Sponsor:  SponsorU
--+--
Changes (by mcs):

 * owner:  tbb-team => mcs
 * status:  new => assigned


--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

[tor-bugs] #19273 [Applications/Tor Browser]: Write C++ patch to replace external applications helper dialog workaround

[Tor Bug Tracker & Wiki]

#19273: Write C++ patch to replace external applications helper dialog 
workaround
--+
 Reporter:  gk|  Owner:  tbb-team
 Type:  task  | Status:  new
 Priority:  High  |  Milestone:
Component:  Applications/Tor Browser  |Version:
 Severity:  Normal|   Keywords:
Actual Points:|  TorBrowserTeam201606
   Points:|  Parent ID:
  Sponsor:  SponsorU  |   Reviewer:
--+
 Part of our deliverable should be to get rid of out clumsy attempts to
 hook the external applications helper dialog. We should patch the Mozilla
 code instead doing trying to fix that from extensions land. This might
 avoid #18090 and other weird side-effects.

--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs