Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Viewer pane fails to recongnize local web contents on Windows #3577

Open
yutannihilation opened this issue Oct 1, 2018 · 18 comments
Open

Viewer pane fails to recongnize local web contents on Windows #3577

yutannihilation opened this issue Oct 1, 2018 · 18 comments
Labels
Milestone

Comments

@yutannihilation
Copy link
Contributor

@yutannihilation yutannihilation commented Oct 1, 2018

System details

RStudio Edition : Desktop
RStudio Version : 1.2.1013
OS Version      :  Windows 10
R Version       :  3.5.1
Locale          : Japanese_Japan.932

Steps to reproduce the problem

reprex::reprex({print(1)})

Describe the problem in detail

With Preview version of RStudio, reprex::reprex() will launch a web browser, not Viewer pane. I fixed once this problem on reprex's side (tidyverse/reprex#75), but now it occurs again. Are there any changes on how to determine a content is local or remote?

Describe the behavior you expected

Open the document in Viewer pane.

@kevinushey

This comment has been minimized.

Copy link
Contributor

@kevinushey kevinushey commented Oct 1, 2018

What is the value of getOption("viewer") in your session?

@yutannihilation

This comment has been minimized.

Copy link
Contributor Author

@yutannihilation yutannihilation commented Oct 1, 2018

Here's the result:

> getOption("viewer")
function (url, height = NULL) 
{
    if (!is.character(url) || (length(url) != 1)) 
        stop("url must be a single element character vector.", 
            call. = FALSE)
    if (identical(height, "maximize")) 
        height <- -1
    if (!is.null(height) && (!is.numeric(height) || (length(height) != 
        1))) 
        stop("height must be a single element numeric vector or 'maximize'.", 
            call. = FALSE)
    invisible(.Call("rs_viewer", url, height))
}
<environment: 0x0000024e428d1288>
@kevinushey

This comment has been minimized.

Copy link
Contributor

@kevinushey kevinushey commented Oct 1, 2018

Strange; I can't reproduce the issue (output shows up in the Viewer pane as expected for me).

One other thing I can think of checking -- what is the output of tempdir() on your system?

Also, can you confirm that the 'viewer' function is indeed being called? E.g. with trace(getOption("viewer")). If not, then you're probably being routed to browseURL (trace(getOption("browser")))

@yutannihilation

This comment has been minimized.

Copy link
Contributor Author

@yutannihilation yutannihilation commented Oct 1, 2018

My tempdir() is

> tempdir()
[1] "C:\\Users\\HIROAK~1\\AppData\\Local\\Temp\\Rtmp6R2FQs"

and by debug()ing reprex::reprex(), I found url is this:

Browse[2]> html_file
C:/Users/hiroaki-yutani/AppData/Local/Temp/Rtmp6R2FQs/reprex2d247669fe/reprex_reprex.html

Neither of trace() worked, but viewer here is definitly getOption("viewer"):

    viewer <- getOption("viewer") %||% utils::browseURL
    viewer(html_file)

https://github.com/tidyverse/reprex/blob/de45a371e3356371fa092a2c927592a6793a25f0/R/reprex.R#L358

@yutannihilation

This comment has been minimized.

Copy link
Contributor Author

@yutannihilation yutannihilation commented Oct 1, 2018

Hmm, I thought there's some chnage around rs_viwer or realPath, but it seems there has been no change for years. Strange...

SEXP rs_viewer(SEXP urlSEXP, SEXP heightSEXP)

Error realPath(const FilePath& filePath, FilePath* pRealPath)

@kevinushey

This comment has been minimized.

Copy link
Contributor

@kevinushey kevinushey commented Oct 1, 2018

I think RStudio is getting confused by the so-called Windows 'short' path name. Compare e.g.

contents <- "<html><body>Hello, world!</body></html>"

dir <- tempfile("html-")
dir.create(dir)
file <- file.path(dir, "reprex.html")
writeLines(contents, con = file)

viewer <- getOption("viewer")

# opens in Viewer
viewer(normalizePath(file, winslash = "/"))

# opens in external browser
viewer(shortPathName(file))

And compare e.g.

> normalizePath(file)
[1] "C:\\Users\\kevin\\AppData\\Local\\Temp\\RtmpGWdb7r\\html-1fd4230037c8\\reprex.html"
> shortPathName(file)
[1] "C:\\Users\\kevin\\AppData\\Local\\Temp\\RTMPGW~1\\HTML-1~3\\REPREX~1.HTM"

This could be fixed on either the RStudio side or the reprex side by calling normalizePath() first on the requested file, I think.

@yutannihilation

This comment has been minimized.

Copy link
Contributor Author

@yutannihilation yutannihilation commented Oct 1, 2018

I'm confused, it seems the opposite of what I found on tidyverse/reprex#75...?

And both viewer(normalizePath(file, winslash = "/")) and viewer(shortPathName(file)) open in external browser on my Environment.

@kevinushey

This comment has been minimized.

Copy link
Contributor

@kevinushey kevinushey commented Oct 1, 2018

That's surprising :-/

Normally, RStudio is supposed to check to see whether the HTML file lives within a sub-directory of the R temporary directory; if it does, RStudio will open it within the Viewer pane.

I'm not sure what else could be causing this. Do you see the same behavior in RStudio v1.1, or does the file properly open in the viewer in that case?

@yutannihilation

This comment has been minimized.

Copy link
Contributor Author

@yutannihilation yutannihilation commented Oct 1, 2018

Do you see the same behavior in RStudio v1.1

Sorry, this happens on RStudio v1.1, too. So, the change is maybe because of some change introduced in reprex package, not in RStudio. Maybe I should close this issue for now, and file a new one again after some investigation... Sorry for the mess.

However, different from your case, none of the following opens in Viewer pane:

viewer(normalizePath(file))
viewer(normalizePath(file, winslash = "/"))
viewer(shortPathName(file))

Only this worked:

viewer(file)
@kevinushey

This comment has been minimized.

Copy link
Contributor

@kevinushey kevinushey commented Oct 1, 2018

This is all pretty surprising to me; I'm not sure what the underlying cause of this behavior could be.

For posterity, how do the file paths differ in each case?

@yutannihilation

This comment has been minimized.

Copy link
Contributor Author

@yutannihilation yutannihilation commented Oct 2, 2018

how do the file paths differ in each case?

I think it's basically same as yours. I've left from in front of my Windows today, so I'll post tomorrow, maybe.

@yutannihilation

This comment has been minimized.

Copy link
Contributor Author

@yutannihilation yutannihilation commented Oct 2, 2018

Here's the result:

dir <- tempfile("html-")
dir.create(dir)
file <- file.path(dir, "reprex.html")
writeLines(contents, con = file)
#> Error in writeLines(contents, con = file): object 'contents' not found

file
#> [1] "C:\\Users\\HIROAK~1\\AppData\\Local\\Temp\\RtmpuCO9hS\\html-244634e38a8/reprex.html"
normalizePath(file)
#> [1] "C:\\Users\\hiroaki-yutani\\AppData\\Local\\Temp\\RtmpuCO9hS\\html-244634e38a8\\reprex.html"
normalizePath(file, winslash = "/")
#> [1] "C:/Users/hiroaki-yutani/AppData/Local/Temp/RtmpuCO9hS/html-244634e38a8/reprex.html"
shortPathName(file)
#> [1] "C:\\Users\\HIROAK~1\\AppData\\Local\\Temp\\RTMPUC~1\\HTML-2~1\\REPREX~1.HTM"

Created on 2018-10-02 by the reprex package (v0.2.1)

@kevinushey

This comment has been minimized.

Copy link
Contributor

@kevinushey kevinushey commented Oct 2, 2018

So in your case, it works with the short path version of your home directory, but not the long-path version? But it doesn't work if other elements in the path are short-path versions?

@kevinushey

This comment has been minimized.

Copy link
Contributor

@kevinushey kevinushey commented Oct 2, 2018

I can reproduce on a user account where the user name gets truncated to the 'short' pathname version. It looks like there's a bug where RStudio doesn't canonicalize the home directory correctly in some cases when it's represented as a short path name.

This might be tricky to get right in RStudio (I don't think we're comfortable fixing this for v1.2) but you should be able to work around it with something like:

short_home <- function(path) {
  path <- normalizePath(path)
  tmpdir <- normalizePath(tempdir())
  if (regexpr(tmpdir, path, fixed = TRUE) == 1)
    path <- sub(tmpdir, tempdir(), path, fixed = TRUE)
  path
}
@yutannihilation

This comment has been minimized.

Copy link
Contributor Author

@yutannihilation yutannihilation commented Oct 3, 2018

Thanks, I didn't think username part matters...

Also thanks for the workaround. I will propose it to reprex package.

@jennybc

This comment has been minimized.

Copy link
Member

@jennybc jennybc commented Oct 19, 2018

@kevinushey Can you give me exact info on what Rstudio regards as the filepath for session temp directory when deciding if a file can be opened in the Viewer pane? I gather I need to do post hoc regex substitution, but I still a bit uncertain re: my target.

@jmcphers

This comment has been minimized.

Copy link
Member

@jmcphers jmcphers commented Jan 3, 2020

The check is here:

bool isSessionTempPath(FilePath filePath)
{
// get the real path
Error error = core::system::realPath(filePath, &filePath);
if (error)
LOG_ERROR(error);
// get the session temp dir real path; needed since the file path above is
// also a real path--e.g. on OS X, it refers to /private/tmp rather than
// /tmp
FilePath tempDir;
error = core::system::realPath(module_context::tempDir(), &tempDir);
if (error)
LOG_ERROR(error);
return filePath.isWithin(tempDir);
}

@kevinushey kevinushey added this to the v1.4 milestone Feb 13, 2020
@kevinushey

This comment has been minimized.

Copy link
Contributor

@kevinushey kevinushey commented Feb 13, 2020

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
5 participants
You can’t perform that action at this time.