Skip to content
This repository has been archived by the owner on Sep 6, 2021. It is now read-only.

Detect inspector protocol version #7127

Closed
jasonsanjose opened this issue Mar 7, 2014 · 10 comments
Closed

Detect inspector protocol version #7127

jasonsanjose opened this issue Mar 7, 2014 · 10 comments

Comments

@jasonsanjose
Copy link
Member

To fix #6830, PR #7008 uses both the deleted CSS.getAllStylesheetsAPI (for Chrome < 34) and the new styleSheetAdded/styleSheetRemoved events.

Users using Chrome >= 34 may see an inconsequential console error in Brackets' dev tools:

// error code -32601 method not found, see http://www.jsonrpc.org/specification#error_object
'CSS.getAllStylesheets' wasn't found ... code: -32601...

We log all inspector protocol errors to the console, even in this case where we expect it to fail in some versions of Chrome. In this case, once we detect the API is missing, we disable future calls during the session and rely only on the new events.

Here are some options so that we don't log the error:

  • Do not call the API if we could detect the protocol version in use. However, the protocol doesn't expose the version at all or what methods it supports. We would have to detect the Chrome version and map that to a revision of protocol.json from http://src.chromium.org/viewvc/blink/trunk/Source/devtools/protocol.json. We could do this client side with some userAgent sniffing or natively if we can read the Chrome version from the native OS. The drawback here is maintaining this mapping for each Chrome version.
  • Filter known error messages/codes out of the console error handler. The drawback here is possibly missing important errors for future API changes.
@redmunds
Copy link
Contributor

redmunds commented Mar 7, 2014

I think we can use chrome://version to get the version. Probably need a new brackets-shell API call.

@peterflynn
Copy link
Member

We can get the version more directly from JS running on the page (e.g. via userAgent) -- is it too late by then to be getting the version?

@jasonsanjose
Copy link
Member Author

@peterflynn no it's not too late, we can do that during the interstitial page.

Just to be extra clear, knowing the chrome version is only half the battle. We still need to have our own mapping of what protocol version maps to what chrome version. I'm not sure that's desirable.

@peterflynn
Copy link
Member

Since there are only two protocol versions we support and there's a clear line in its version history where Chrome switched from one to the other, it doesn't seem that bad to me. It's not like a caniuse matrix with lots of different browsers hitting the mark at different times or anything...

@dangoor
Copy link
Contributor

dangoor commented Mar 19, 2014

Low priority to @redmunds

@njx
Copy link

njx commented Apr 8, 2014

We should note this in the release notes for 38, since it shows up as a console error and people might think something is wrong.

@njx
Copy link

njx commented Apr 8, 2014

I just went ahead and did that.

@peterflynn
Copy link
Member

FBNC @jasonsanjose since the PR has now landed. I'll remove the release notes entry.

@redmunds
Copy link
Contributor

redmunds commented May 7, 2014

@jasonsanjose Are you around to verify this today?

@jasonsanjose
Copy link
Member Author

Confirmed. Closing.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

5 participants