Skip to content
This repository has been archived by the owner on Dec 11, 2019. It is now read-only.

Local file doesn't open in Brave, DDG search is run instead #5828

Closed
echosa opened this issue Nov 23, 2016 · 10 comments
Closed

Local file doesn't open in Brave, DDG search is run instead #5828

echosa opened this issue Nov 23, 2016 · 10 comments

Comments

@echosa
Copy link
Contributor

echosa commented Nov 23, 2016

Did you search for similar issues before submitting this one?

Yes

Describe the issue you encountered:

In Safari, I started playing Zork here: http://www.ifiction.org/games/playz.php?cat=2&game=386&mode=html

According to their FAQ, you save your game by saving the HTML page to your computer. I did that, and when I open it in Safari, it correctly opens. When I make a move, it correctly takes me back to the actual ifiction.org page so I can continue playing.

When I open the file in Brave, either by double-clicking the file or drag-and-dropping the file onto the tab bar, Brave runs a DDG search for the file location on my computer.

It's weird to explain, so here's a screecast of what's going on. First I double-click the file to open in Brave. Brave attempts a DDG search. Then I open the file in Safari (opens a file:/// url) then make a move in the game (takesme to ifiction.org).

https://cl.ly/0v3Y1O3L402j

Expected behavior:

The file should open in Brave and work like it does in Safari.

  • Platform (Win7, 8, 10? macOS? Linux distro?):

OS X 10.11.6

  • Brave Version:

0.12.10

  • Steps to reproduce:
    1. Download the zip file I've attached and get the HTML file out of it, or start your own ifiction.org game and save it according to their FAQ.
    2. Open the HTML file in Brave.

iFiction - Game - Dungeon.zip

@echosa
Copy link
Contributor Author

echosa commented Nov 24, 2016

Additional note: if I open the file via File -> Open File it works correctly.

@cndouglas
Copy link

I've also noticed this with other file types, such as PDFs.

@NejcZdovc
Copy link
Contributor

NejcZdovc commented Dec 18, 2016

I think that I found the problem. Problem is in the path that is send to the browser.

If you drag and drop file with path something like /Users/user/Desktop/Screen-Shot-2016-12-14-at-13.39.15.png it will work, but it will not work for path like /Users/user/Desktop/Screen Shot 2016-12-14 at 13.39.15.png, for paths that are not encoded (containing spaces for example).

@bsclifton @bbondy @luixxiul Any suggestion where to look in the code to fix this problem?

@bsclifton
Copy link
Member

@NejcZdovc I believe this is the code you'd want to look at:
https://github.com/brave/browser-laptop/blob/master/app/cmdLine.js#L63

This code should be getting executed on all platforms (I've only personally verified on Windows) after the extension handler finds Brave to be a match for the file being double clicked. It passes the input in this format:
Brave -- "all arguments here"

I think the resolution would be to join all remaining argv indices together by spaces, starting at index + 1

@NejcZdovc
Copy link
Contributor

@bsclifton Thank you for a pointer. How can I easily debug this process? The only way that I found out is by creating build package and then test it (problem is that you don't have debug options in package). I tried to fix it without debugging but with no luck.

@NejcZdovc
Copy link
Contributor

We need to do two things I think. First merge arguments and them encode them. Second thing I already implemented in #6404 for drag and drop and it's working correctly.

@bsclifton
Copy link
Member

@NejcZdovc this is not the easiest to debug. Basically, you'd need to make your changes, then run npm run build-package to package up the executable. At that point, you should be able to copy the executable generated over main version you use

You are 100% correct though- we will want to sanitize the arguments also

@NejcZdovc
Copy link
Contributor

@bsclifton will try to fix it with building, hope will figure it out without debug 📦

NejcZdovc added a commit to NejcZdovc/browser-laptop that referenced this issue Dec 26, 2016
Fixes #brave#5828

Auditors: @bsclifton @echosa

Test Plan:
- Build brave
- Open local file with brave (double click)
- Local file should be opened
NejcZdovc added a commit to NejcZdovc/browser-laptop that referenced this issue Dec 26, 2016
Fixes #brave#5828

Auditors: @bsclifton @echosa

Test Plan:
- Build brave
- Open local file with brave (double click)
- Local file should be opened
@NejcZdovc
Copy link
Contributor

@bsclifton as far as I can tell the problem was only in encoding so it was a easy fix when I found out where the code is located 😄

@bsclifton bsclifton added this to the 0.13.1 milestone Dec 29, 2016
@NejcZdovc NejcZdovc self-assigned this Jan 9, 2017
@bsclifton
Copy link
Member

Fixed with #6424 which was merged into branch 0.13.1-branch.

bsclifton pushed a commit that referenced this issue Jan 14, 2017
Fixes ##5828

Auditors: @bsclifton @echosa

Test Plan:
- Build brave
- Open local file with brave (double click)
- Local file should be opened
bsclifton pushed a commit that referenced this issue Jan 17, 2017
Fixes ##5828

Auditors: @bsclifton @echosa

Test Plan:
- Build brave
- Open local file with brave (double click)
- Local file should be opened
bsclifton pushed a commit that referenced this issue Jan 17, 2017
Fixes ##5828

Auditors: @bsclifton @echosa

Test Plan:
- Build brave
- Open local file with brave (double click)
- Local file should be opened
bsclifton pushed a commit that referenced this issue Jan 18, 2017
Fixes ##5828

Auditors: @bsclifton @echosa

Test Plan:
- Build brave
- Open local file with brave (double click)
- Local file should be opened
bsclifton pushed a commit that referenced this issue Jan 20, 2017
Fixes ##5828

Auditors: @bsclifton @echosa

Test Plan:
- Build brave
- Open local file with brave (double click)
- Local file should be opened
NejcZdovc added a commit to NejcZdovc/browser-laptop that referenced this issue Jan 23, 2017
Fixes #brave#5828

Auditors: @bsclifton @echosa

Test Plan:
- Build brave
- Open local file with brave (double click)
- Local file should be opened
bsclifton pushed a commit that referenced this issue Jan 23, 2017
Fixes ##5828

Auditors: @bsclifton @echosa

Test Plan:
- Build brave
- Open local file with brave (double click)
- Local file should be opened
bsclifton pushed a commit that referenced this issue Jan 24, 2017
Fixes ##5828

Auditors: @bsclifton @echosa

Test Plan:
- Build brave
- Open local file with brave (double click)
- Local file should be opened
bsclifton pushed a commit that referenced this issue Jan 25, 2017
Fixes ##5828

Auditors: @bsclifton @echosa

Test Plan:
- Build brave
- Open local file with brave (double click)
- Local file should be opened
bsclifton pushed a commit that referenced this issue Jan 25, 2017
Fixes ##5828

Auditors: @bsclifton @echosa

Test Plan:
- Build brave
- Open local file with brave (double click)
- Local file should be opened
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.