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

Add getSourceFromBrowser for CSS documents #26

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

sebaslv
Copy link
Contributor

@sebaslv sebaslv commented Oct 3, 2014

Adds LiveCSSDocument.getSourceFromBrowser() by adding getStylesheetText() to protocol layer to be used as the underlying infrastructure for CSS Editing unit tests. This PR also includes an initial test for CSS Editing.

text = sheet.ownerNode.innerText;
}
// if it was not already 'reloaded'
if (!sheet.disabled && sheet.href === msg.params.url) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

The comments make it sound like these blocks are mutually exclusive, so shouldn't this be else if (...) ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Actually yes, I'll change that.

@redmunds
Copy link
Collaborator

redmunds commented Oct 7, 2014

I'm seeing new unit test fail:

should push changes through browser connection

TypeError: Cannot call method 'replace' of undefined
    at fixSpaces (file:///C:/Users/redmunds/dev/github/brackets-shell/Release/dev/src/extensions/dev/brackets-livedev2/unittests.js:54:24)
    at null.<anonymous> (file:///C:/Users/redmunds/dev/github/brackets-shell/Release/dev/src/extensions/dev/brackets-livedev2/unittests.js:222:28)
    at jasmine.Block.execute (file:///C:/Users/redmunds/dev/github/brackets-shell/Release/dev/test/thirdparty/jasmine-core/jasmine.js:1064:17)
    at jasmine.Queue.next_ (file:///C:/Users/redmunds/dev/github/brackets-shell/Release/dev/test/thirdparty/jasmine-core/jasmine.js:2096:31)
    at onComplete (file:///C:/Users/redmunds/dev/github/brackets-shell/Release/dev/test/thirdparty/jasmine-core/jasmine.js:2092:18)
    at jasmine.WaitsForBlock.execute (file:///C:/Users/redmunds/dev/github/brackets-shell/Release/dev/test/thirdparty/jasmine-core/jasmine.js:2576:5)
    at file:///C:/Users/redmunds/dev/github/brackets-shell/Release/dev/test/thirdparty/jasmine-core/jasmine.js:2590:12

@redmunds
Copy link
Collaborator

redmunds commented Oct 7, 2014

Done with code review.

@sebaslv
Copy link
Contributor Author

sebaslv commented Oct 7, 2014

It seems the failure is related to getStylesheetText not retrieving the code from the browser side. I'm not able to make it fail even running on different browsers (IE, Firefox and Chrome). Which browser are you using to run it?

@redmunds
Copy link
Collaborator

redmunds commented Oct 7, 2014

I'm on Windows 7 and my default browser (where tests are being run) is Firefox.

@sebaslv
Copy link
Contributor Author

sebaslv commented Oct 20, 2014

Added a couple of changes to the implementation of getStylesheetText. Tests are passing in Firefox and Chrome on my environment (Windows 8) against master branch of Brackets. Could you please check them now? Thanks!

@redmunds
Copy link
Collaborator

We're trying to finish up Brackets 1.0 this week and next week, so not sure when I'll get to it.

@redmunds
Copy link
Collaborator

I'm still seeing same error on Win7/Firefox, which I'm also seeing in pull request #28

@redmunds
Copy link
Collaborator

I verified that tests pass on MacOSX 10.8/Safari.

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.

2 participants