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

[CLOSED] Detect inspector protocol version #6449

Open
core-ai-bot opened this issue Aug 30, 2021 · 10 comments
Open

[CLOSED] Detect inspector protocol version #6449

core-ai-bot opened this issue Aug 30, 2021 · 10 comments

Comments

@core-ai-bot
Copy link
Member

Issue by jasonsanjose
Friday Mar 07, 2014 at 19:39 GMT
Originally opened as adobe/brackets#7127


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.
@core-ai-bot
Copy link
Member Author

Comment by redmunds
Friday Mar 07, 2014 at 22:27 GMT


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

@core-ai-bot
Copy link
Member Author

Comment by peterflynn
Friday Mar 07, 2014 at 22:57 GMT


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?

@core-ai-bot
Copy link
Member Author

Comment by jasonsanjose
Friday Mar 07, 2014 at 23:04 GMT


@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.

@core-ai-bot
Copy link
Member Author

Comment by peterflynn
Friday Mar 07, 2014 at 23:06 GMT


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...

@core-ai-bot
Copy link
Member Author

Comment by dangoor
Wednesday Mar 19, 2014 at 17:59 GMT


Low priority to@redmunds

@core-ai-bot
Copy link
Member Author

Comment by njx
Tuesday Apr 08, 2014 at 19:45 GMT


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.

@core-ai-bot
Copy link
Member Author

Comment by njx
Tuesday Apr 08, 2014 at 19:47 GMT


I just went ahead and did that.

@core-ai-bot
Copy link
Member Author

Comment by peterflynn
Thursday May 01, 2014 at 23:40 GMT


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

@core-ai-bot
Copy link
Member Author

Comment by redmunds
Wednesday May 07, 2014 at 17:11 GMT


@jasonsanjose Are you around to verify this today?

@core-ai-bot
Copy link
Member Author

Comment by jasonsanjose
Wednesday May 07, 2014 at 18:40 GMT


Confirmed. Closing.

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

No branches or pull requests

1 participant