Use styleSheetAdded and styleSheetRemoved, replaces getAllStylesheets #7008

jasonsanjose · 2014-02-26T00:18:11Z

Fix for #6830

Add styleSheetAdded and styleSheetRemoved event handling to inspector protocol
Fallback to getAllStylesheets for Chrome < 34

Fix for #6912

Use styleSheetAdded to keep styleSheetId values up to date to fix "No style sheet with given id found, code: -32000" when pushing change events from Brackets to Chrome

…ylesheets API

jasonsanjose · 2014-02-26T00:39:11Z

src/LiveDevelopment/LiveDevelopment.js

Since getStylesheetURLs depended on getAllStylesheets, most of this code now moves into the styleSheetAdded event handler.

jasonsanjose · 2014-02-26T01:07:32Z

Doh. I just realized that we should check for the original API in case the user is using an older version of Chrome.

jasonsanjose · 2014-02-26T02:18:26Z

Added fallback for getAllStyleSheets and duplicate detection if both the deleted API and the events are working. Tested on Mac 10.9 with Chrome 33.

Even though Chrome 33 should support both, I found that the styleSheetAdded and styleSheetRemoved events were not consistently fired by Chrome. The fallback was necessary in my testing otherwise live CSS editing would be broken.

jasonsanjose · 2014-02-26T02:19:17Z

Somewhat related to these changes is PR #7010 to update the inspector protocol to the latest stable version.

njx · 2014-02-26T17:43:03Z

src/LiveDevelopment/Agents/CSSAgent.js

Will this dump out an error in the console if the API doesn't exist? I wonder if we should bother doing this, since the events seem to have been around for awhile, and I don't think we need to support people who don't regularly update Chrome. (Although I guess we don't know exactly which version they were introduced in.)

It will yes. We can suppress a subset of protocol errors, particularly this one where the method is not defined. The basic set of error codes are defined here http://www.jsonrpc.org/specification#error_object. I can't find any documentation for Chrome that defines implementation-specific error codes.

…rotocol

jasonsanjose · 2014-02-26T20:04:06Z

src/LiveDevelopment/Agents/CSSAgent.js

Allow enable during LiveDevelopment _onInterstitialPageLoad(). The old behavior would wait all the way until loadEventFired to enable the CSS domain. This also allows loadAgents() to complete faster since we no longer wait for the sequence of (1) loadEventFired, (2) enable and (3) getAllStylesheets.

redmunds · 2014-02-27T19:30:54Z

I'm seeing the this LiveDevelopment unit test fail every time on Win7 (both HD and SSD) and Chrome 33:

"should reload the page when editing a non-live document"

timeout: timed out after 1000 msec waiting for success [FILE_SAVE]

UPDATE: seeing same failure with Win7 (SSD) with Chrome 34.0.1847.11 beta-m.

redmunds · 2014-02-27T20:13:14Z

FYI, Win7 (HD) with Chrome 35 (canary) is a train wreck (7 of 9 fail). Getting "External Changes: simple1.css has been modified on disk, but also has unsaved changes in Brackets" warning dialog.

redmunds · 2014-02-27T20:46:22Z

src/LiveDevelopment/Agents/CSSAgent.js

Since code no longer checks if (_urlToStyle) then _urlToStyle should be initialized to _urlToStyle = {} on line 43 to be safe. Same for _styleSheetIdToUrl.

redmunds · 2014-02-27T21:24:05Z

There are a lot of conflicts with PR #6889 -- should these be merged?

…interim updates cause spurious added/removed events.

jasonsanjose · 2014-02-28T00:22:45Z

@redmunds Looking at the unit test failures now. Will send an update later today.

…for CSSDocument.

…ackets into jasonsanjose/issue-6830

redmunds · 2014-02-28T21:03:33Z

src/LiveDevelopment/Agents/CSSAgent.js

No longer necessary to check for existence of _urlToStyle object.

jasonsanjose · 2014-02-28T21:04:02Z

@redmunds let me know if you see a unit test failure at the end of the live dev suite due to removeTempDirectory. I can only reproduce on windows, not mac. I've scrubbed this code all morning looking for some material change that would maybe cause file watchers to hang on to a ref, which would maybe cause the error during cleanup, but I don't see anything.

redmunds · 2014-02-28T21:10:34Z

@jasonsanjose Yes, I just saw it on Win7 (SSD) but not on Win7 (HD).

redmunds · 2014-02-28T21:16:03Z

src/LiveDevelopment/LiveDevelopment.js

This comment seems out-of-place since the 2 things listed are not done here. The one item that is done here ("agent loading") was removed from the comment.

Good point. Removed the whole thing.

redmunds · 2014-02-28T22:06:37Z

@jasonsanjose I tried to manually delete the test/temp folder and got the message: "The action can't be completed because the folder or file is open in another program". I shutdown Brackets and could then remove the folder. Now the tests pass. Interesting because I know that I had a clean git status when I started.

…ackets into jasonsanjose/issue-6830

jasonsanjose · 2014-03-04T17:42:49Z

@redmunds cleaned up those last few comments. Still no luck with the removeTempDirectory problem.

redmunds · 2014-03-04T17:51:43Z

@jasonsanjose Do you have a repeatable recipe for the removeTempDirectory problem? If so, I can take a look.

jasonsanjose · 2014-03-04T18:03:02Z

Unfortunately no. It happens about 90% of the time I run the live dev test suite.

…6830

redmunds · 2014-03-05T00:03:50Z

I'm getting an exception in HighlightAgent unit test on Win7 (SSD) with Chrome 33:

should redraw highlights when the document changes

TypeError: Cannot call method 'done' of undefined
    at CSSDocumentModule.CSSDocument._updateBrowser (file:///C:/Users/redmunds/dev/github/brackets-shell/Release/dev/src/LiveDevelopment/Documents/CSSDocument.js:136:27)
    at CSSDocument.onChange (file:///C:/Users/redmunds/dev/github/brackets-shell/Release/dev/src/LiveDevelopment/Documents/CSSDocument.js:208:14)
    at Document.o.event.dispatch (file:///C:/Users/redmunds/dev/github/brackets-shell/Release/dev/src/thirdparty/jquery-2.1.0.min.js:3:6055)
    at Document.r.handle (file:///C:/Users/redmunds/dev/github/brackets-shell/Release/dev/src/thirdparty/jquery-2.1.0.min.js:3:2830)
    at Object.o.event.trigger (file:///C:/Users/redmunds/dev/github/brackets-shell/Release/dev/src/thirdparty/jquery-2.1.0.min.js:3:5163)
    at o.fn.extend.triggerHandler (file:///C:/Users/redmunds/dev/github/brackets-shell/Release/dev/src/thirdparty/jquery-2.1.0.min.js:3:11081)
    at Document.docToShim._handleEditorChange (file:///C:/Users/redmunds/dev/github/brackets-shell/Release/dev/test/spec/SpecRunnerUtils.js:334:21)
    at Editor.o.event.dispatch (file:///C:/Users/redmunds/dev/github/brackets-shell/Release/dev/src/thirdparty/jquery-2.1.0.min.js:3:6055)
    at Editor.r.handle (file:///C:/Users/redmunds/dev/github/brackets-shell/Release/dev/src/thirdparty/jquery-2.1.0.min.js:3:2830)
    at Object.o.event.trigger (file:///C:/Users/redmunds/dev/github/brackets-shell/Release/dev/src/thirdparty/jquery-2.1.0.min.js:3:5163)

jasonsanjose · 2014-03-05T00:24:57Z

@redmunds fixed the HighlightAgent test

redmunds · 2014-03-05T01:22:04Z

src/LiveDevelopment/LiveDevelopment.js

Isn't the path null if it's not an external stylesheet (i.e. embedded) ?

redmunds · 2014-03-05T02:00:40Z

@jasonsanjose I can still hit the removeTempDirectory problem, but not very often. When I hit it, I manually delete test/temp folder, and it's ok for a while.

But, we need to get this in master so people start using it. We also need to figure out which pieces of #6889 we need to take. Merging.

Use styleSheetAdded and styleSheetRemoved, replaces getAllStylesheets

jasonsanjose added 2 commits February 25, 2014 16:16

use styleSheetAdded and styleSheetRemoved to replace deleted getAllSt…

04b015e

…ylesheets API

fix related doc ref count errors

e2ef6eb

jasonsanjose reviewed Feb 26, 2014
View reviewed changes

fallback to getAllStyleSheets if available

0905ca4

njx reviewed Feb 26, 2014
View reviewed changes

dangoor assigned JeffryBooher Feb 26, 2014

add error handling when getAllStyleSheets is not found in inspector p…

02e2b10

…rotocol

jasonsanjose reviewed Feb 26, 2014
View reviewed changes

do not change live dev status for inspector errors

ab40ce0

jasonsanjose mentioned this pull request Feb 26, 2014

Spurious Inspector styleSheetAdded/styleSheetRemoved events causes live preview to stop working #6912

Closed

redmunds mentioned this pull request Feb 27, 2014

Live Preview fixes #6889

Merged

redmunds reviewed Feb 27, 2014
View reviewed changes

jasonsanjose added 2 commits February 27, 2014 14:52

account for styleSheetId changes after a CSSDocument reloads

d2a8a82

remove styleSheetRemoved handler. handle weird chrome behavior where …

87bb739

…interim updates cause spurious added/removed events.

jasonsanjose mentioned this pull request Feb 28, 2014

Live preview error when editing code #5687

Closed

fix jslint error. Fix getSourceFromBrowser response. Remove debounce …

24d3cf2

…for CSSDocument.

JeffryBooher removed their assignment Feb 28, 2014

Merge branch 'jasonsanjose/issue-6830' of https://github.com/adobe/br…

4ac3e07

…ackets into jasonsanjose/issue-6830

redmunds reviewed Feb 28, 2014
View reviewed changes

jasonsanjose added 2 commits March 4, 2014 09:40

code review comments

00dde93

Merge branch 'jasonsanjose/issue-6830' of https://github.com/adobe/br…

85b5d8c

…ackets into jasonsanjose/issue-6830

jasonsanjose added 2 commits March 4, 2014 15:04

Merge remote-tracking branch 'origin/master' into jasonsanjose/issue-…

d1a6752

…6830

fix async call to getLiveDocForPath

3cee9fe

jasonsanjose mentioned this pull request Mar 4, 2014

Wait time for live development to start is too short #6122

Closed

update unit test spy for API change

ac80eca

redmunds reviewed Mar 5, 2014
View reviewed changes

src/LiveDevelopment/LiveDevelopment.js

Copy link

Contributor

redmunds Mar 5, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't the path null if it's not an external stylesheet (i.e. embedded) ?

redmunds added a commit that referenced this pull request Mar 5, 2014

Merge pull request #7008 from adobe/jasonsanjose/issue-6830

d8f8177

Use styleSheetAdded and styleSheetRemoved, replaces getAllStylesheets

redmunds merged commit d8f8177 into master Mar 5, 2014

TomMalbran deleted the jasonsanjose/issue-6830 branch March 5, 2014 02:11

TomMalbran restored the jasonsanjose/issue-6830 branch March 5, 2014 02:12

jasonsanjose deleted the jasonsanjose/issue-6830 branch March 5, 2014 03:04

oslego mentioned this pull request Mar 6, 2014

Live Preview Error - Chrome 34 removes CSS.getAllStylesheets() command #6830

Closed

This was referenced Mar 7, 2014

Detect inspector protocol version #7127

Closed

"Unable to load Live Development page" error shortly after starting Live Preview #7097

Closed

njx mentioned this pull request May 2, 2014

[server smokes] In-memory CSS changes not applied when saving server HTML file #7606

Closed

core-ai-bot mentioned this pull request Aug 30, 2021

[CLOSED] Use styleSheetAdded and styleSheetRemoved, replaces getAllStylesheets brackets-archive/bracketsIssues#6345

Open

Use styleSheetAdded and styleSheetRemoved, replaces getAllStylesheets #7008

Use styleSheetAdded and styleSheetRemoved, replaces getAllStylesheets #7008

Uh oh!

Conversation

jasonsanjose commented Feb 26, 2014

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jasonsanjose commented Feb 26, 2014

Uh oh!

jasonsanjose commented Feb 26, 2014

Uh oh!

jasonsanjose commented Feb 26, 2014

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

redmunds commented Feb 27, 2014

Uh oh!

redmunds commented Feb 27, 2014

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

redmunds commented Feb 27, 2014

Uh oh!

jasonsanjose commented Feb 28, 2014

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jasonsanjose commented Feb 28, 2014

Uh oh!

redmunds commented Feb 28, 2014

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

redmunds commented Feb 28, 2014

Uh oh!

jasonsanjose commented Mar 4, 2014

Uh oh!

redmunds commented Mar 4, 2014

Uh oh!

jasonsanjose commented Mar 4, 2014

Uh oh!

redmunds commented Mar 5, 2014

Uh oh!

jasonsanjose commented Mar 5, 2014

Uh oh!

Choose a reason for hiding this comment

Uh oh!

redmunds commented Mar 5, 2014

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants