search

GoogleChromeLabs / carlo

Web rendering surface for Node applications
Apache License 2.0
9.31k stars 309 forks source link

Wrong chrome path priority? #99

Closed xyc closed 5 years ago

xyc commented 5 years ago

Looks that sorting exec path priorities places path with greater weight in the beginning. https://github.com/GoogleChromeLabs/carlo/blob/09416d8a49ed6df88fa2f1f62e35c567f072340c/lib/find_chrome.js#L145 But it pops the last (least weight). Is this intentional? https://github.com/GoogleChromeLabs/carlo/blob/09416d8a49ed6df88fa2f1f62e35c567f072340c/lib/find_chrome.js#L60

pavelfeldman commented 5 years ago

I don't think this priority is used at all after the boolean canary flag introduction. Or are you experiencing any issues because of this sorting? Do you have multiple Chrome installs?

xyc commented 5 years ago

Yes, I have multiple Chrome installs. I expect it to pick up /Applications/Google Chrome.app but it pick up the lower priority one (which is not the latest version). Reversing the weight comparator (edit: a.weight - b.weight) would resolve it.

pavelfeldman commented 5 years ago

Fixed in https://github.com/GoogleChromeLabs/carlo/commit/e33f9d13da41b13372aa6ba20742f5e240412b39

xyc commented 5 years ago

👍 thank you!