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

Workaround for #10124: change the title on close. #10129

Merged
merged 2 commits into from
Dec 9, 2014

Conversation

busykai
Copy link
Contributor

@busykai busykai commented Dec 9, 2014

On Mac/OSX alert causes system modal dialog which prevents any interaction with
the browser and navigating away. This was causing unit tests to fail.

So instead of showing alert, change the title of the document to indicate that
the Live Preview session was closed. A more clear UI/UX definition is needed
for definitive solution.

Fixes #10124.

CC @redmunds

On Mac/OSX alert causes system modal dialog which prevents any interaction with
the browser and navigating away. This was causing unit tests to fail.

So instead of showing alert, change the title of the document to indicate that
the Live Preview session was closed. A more clear UI/UX definition is needed
for definitive solution.
@redmunds
Copy link
Contributor

redmunds commented Dec 9, 2014

@busykai In original extension, there's a custom overlay message displayed when browser is closed that I like -- why was that dropped?

@redmunds
Copy link
Contributor

redmunds commented Dec 9, 2014

I'm still seeing 1 MultiBrowser (experimental) - LiveDevelopment unit test fail:

"should push in memory css changes made before the session starts"

TypeError: Cannot read property 'getSourceFromBrowser' of undefined
    at null.<anonymous> (file://localhost/Users/randyedmunds/github/brackets-shell/xcodebuild/Release/Brackets.app/Contents/dev/test/spec/LiveDevelopmentMultiBrowser-test.js:289:28)
    at jasmine.Block.execute (file://localhost/Users/randyedmunds/github/brackets-shell/xcodebuild/Release/Brackets.app/Contents/dev/test/thirdparty/jasmine-core/jasmine.js:1064:17)
    at jasmine.Queue.next_ (file://localhost/Users/randyedmunds/github/brackets-shell/xcodebuild/Release/Brackets.app/Contents/dev/test/thirdparty/jasmine-core/jasmine.js:2096:31)
    at onComplete (file://localhost/Users/randyedmunds/github/brackets-shell/xcodebuild/Release/Brackets.app/Contents/dev/test/thirdparty/jasmine-core/jasmine.js:2092:18)
    at jasmine.WaitsForBlock.execute (file://localhost/Users/randyedmunds/github/brackets-shell/xcodebuild/Release/Brackets.app/Contents/dev/test/thirdparty/jasmine-core/jasmine.js:2576:5)
    at file://localhost/Users/randyedmunds/github/brackets-shell/xcodebuild/Release/Brackets.app/Contents/dev/test/thirdparty/jasmine-core/jasmine.js:2590:12
timeout: timed out after 10000 msec waiting for Browser to sync changes

@busykai
Copy link
Contributor Author

busykai commented Dec 9, 2014

@redmunds which Safari version do you use? I have 8.0.2 (10600.3.8.2), running on OSX 10.10.

@redmunds
Copy link
Contributor

redmunds commented Dec 9, 2014

I have Safari Version 6.2 (8537.85.10.17.1) on Mac OS X 10.8.5 .

@busykai
Copy link
Contributor Author

busykai commented Dec 9, 2014

OK. Let me try to find a similar Mac... If I won't be able to resolve it today, we'll have to turn off that test.

As for overlay, I am not exactly sure why it was dropped. I can add it back. We never knew, however, how this implementation should indicate it's been closed. Changing title in addition to the overlay might be actually a good idea.

    - Exclude failing test.
    - Add full screen overlay on Live Preview close in browser.
@busykai
Copy link
Contributor Author

busykai commented Dec 9, 2014

Got it reproduced on OSX 10.9.4 with Safari 7.0.5. However, I think the issue is not the version, but how fast is the machine itself. The issue is that the css is not yet registered as related document and LiveDevelopment.getLiveDocForPath. I'm going to exclude it for now and make sure it fixed ASAP.

The overlay in the original extension was not nice, actually, but I've reworked it a little bit and included here.

@redmunds
Copy link
Contributor

redmunds commented Dec 9, 2014

FWIW, this is a 2 year old MacBook Pro Retina, so it's fast. I just kept the version at 10.8 for testing coverage. Maybe it's an OS difference.

Anyway, looks good. Merging.

redmunds added a commit that referenced this pull request Dec 9, 2014
@redmunds redmunds merged commit 0ebf373 into master Dec 9, 2014
@redmunds redmunds deleted the kai/fix-mb-livedev-test-failures branch December 9, 2014 21:41
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Unit Tests] MultiBrowser tests fail on Mac/Safari
2 participants