Skip to content

Conversation

@cderv
Copy link
Collaborator

@cderv cderv commented May 29, 2022

This closes #972

  • It will add search for EDGE on windows, in addition of Chrome to that our pupeteer related feature works.
  • It fixes an issue with the registry key return by search in HKCR: Command line argument where returned and we need only program path.
  • I added a test existence for program path returned to avoid calling a non-working program.

I tested interactively on my Windows 11 using default Diagram example of Quarto (https://quarto.org/docs/authoring/diagrams.html)

I did not add that yet, but we probably would like to add tests specific for Windows for this part of the code.

@cscheid do you think it would be a good idea to add a check to http://localhost:<remote_port>/json/version for a minimal version requirement ?

Also, for this MSEDGE feature, I am just wondering how to handle possible older version of EDGE if we want to fail on them gracefully. I would need to find an old Windows system to test that.

@jjallaire jjallaire merged commit 896bef2 into main May 29, 2022
@cscheid
Copy link
Collaborator

cscheid commented May 29, 2022

@cscheid do you think it would be a good idea to add a check to http://localhost:<remote_port>/json/version for a minimal version requirement ?

That would be great, except that I don't know what are the actual minimum requirements in order for the features we need to be available. Do you know where to find that?

@cderv cderv deleted the msedge-chromium branch May 30, 2022 12:27
@cderv
Copy link
Collaborator Author

cderv commented Jun 2, 2022

I did not found them either. Usually I refer to this https://peter.sh/experiments/chromium-command-line-switches/ but it has not been updated since some time, and not easy to know when a command line appeared.

In one of my previous project we directly checked that the http://localhost:/json/protocol contained the required method we needed.

But Does the pupeeter version we are using has a minimal requirement ?

@cscheid
Copy link
Collaborator

cscheid commented Jun 2, 2022

Currently, quarto tools install chromium installs the latest version supported by puppeteer:

async function supportedRevision(): Promise<string> {
return (await getPuppeteer())._preferredRevision;
}

@cderv
Copy link
Collaborator Author

cderv commented Jun 2, 2022

So let's leave as is then : We try to use Chrome and Edge if present. If for some reason there is an issue (for version reason for example), we would recommand quarto tools install chromium that we know will work.

if we stumble upon a minimal version requirement in the future, we could add it to fail more gracefully

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Use Microsoft Edge for rendering (on Windows)

4 participants