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

shell: Fix shell exceptions on Internet Explorer #7375

Merged
merged 1 commit into from Aug 2, 2017

Conversation

Projects
None yet
2 participants
@stefwalter
Copy link
Contributor

commented Jul 26, 2017

Simply clicking from one screen to another in Internet Explorer
results in exceptions and Ooops being displayed. This is due
to differences in behavior on what is acessible and which
properties are available on certain events.

@stefwalter

This comment has been minimized.

Copy link
Contributor Author

commented Jul 26, 2017

This seems broken:

Connection to 127.0.0.2 closed by remote host.
Page error: TypeError: undefined is not a function (evaluating 'o.window.postMessage(n, h)')
http://127.0.0.2:9092/cockpit/$56c3ae8b8f73d57ee6bfc783ce7e466c198d3e7d/shell/index.js 1958 hint
http://127.0.0.2:9092/cockpit/$56c3ae8b8f73d57ee6bfc783ce7e466c198d3e7d/shell/index.js 2099 current_frame
http://127.0.0.2:9092/cockpit/$56c3ae8b8f73d57ee6bfc783ce7e466c198d3e7d/shell/index.js 1671 g
http://127.0.0.2:9092/cockpit/$56c3ae8b8f73d57ee6bfc783ce7e466c198d3e7d/shell/index.js 1600 p
http://127.0.0.2:9092/cockpit/$56c3ae8b8f73d57ee6bfc783ce7e466c198d3e7d/shell/index.js 1723 
http://127.0.0.2:9092/cockpit/$56c3ae8b8f73d57ee6bfc783ce7e466c198d3e7d/base1/jquery.js 2460 dispatch
http://127.0.0.2:9092/cockpit/$56c3ae8b8f73d57ee6bfc783ce7e466c198d3e7d/base1/jquery.js 2274 handle
http://127.0.0.2:9092/cockpit/$56c3ae8b8f73d57ee6bfc783ce7e466c198d3e7d/base1/jquery.js 2414 trigger
http://127.0.0.2:9092/cockpit/$56c3ae8b8f73d57ee6bfc783ce7e466c198d3e7d/base1/jquery.js 2791 triggerHandler
http://127.0.0.2:9092/cockpit/$56c3ae8b8f73d57ee6bfc783ce7e466c198d3e7d/shell/index.js 74 o
http://127.0.0.2:9092/cockpit/$56c3ae8b8f73d57ee6bfc783ce7e466c198d3e7d/shell/index.js 169 overlay
http://127.0.0.2:9092/cockpit/$56c3ae8b8f73d57ee6bfc783ce7e466c198d3e7d/shell/index.js 233 s
http://127.0.0.2:9092/cockpit/$56c3ae8b8f73d57ee6bfc783ce7e466c198d3e7d/shell/index.js 255 n
http://127.0.0.2:9092/cockpit/$56c3ae8b8f73d57ee6bfc783ce7e466c198d3e7d/shell/index.js 323 connect
http://127.0.0.2:9092/cockpit/$56c3ae8b8f73d57ee6bfc783ce7e466c198d3e7d/shell/index.js 321 

@martinpitt martinpitt changed the title shell: Fix shell exceptions on Internet Explorer WIP: shell: Fix shell exceptions on Internet Explorer Jul 27, 2017

@stefwalter stefwalter changed the title WIP: shell: Fix shell exceptions on Internet Explorer shell: Fix shell exceptions on Internet Explorer Jul 28, 2017

@stefwalter stefwalter force-pushed the stefwalter:shell-broken-ie branch from a8e12e2 to 225ee03 Jul 28, 2017

stefwalter added a commit to stefwalter/cockpit that referenced this pull request Jul 28, 2017

shell: Fix shell exceptions on Internet Explorer
Simply clicking from one screen to another in Internet Explorer
results in exceptions and Ooops being displayed. This is due
to differences in behavior on what is acessible and which
properties are available on certain events.

Closes cockpit-project#7375

@stefwalter stefwalter force-pushed the stefwalter:shell-broken-ie branch from 225ee03 to 61abeb2 Jul 28, 2017

stefwalter added a commit to stefwalter/cockpit that referenced this pull request Jul 28, 2017

shell: Fix shell exceptions on Internet Explorer
Simply clicking from one screen to another in Internet Explorer
results in exceptions and Ooops being displayed. This is due
to differences in behavior on what is acessible and which
properties are available on certain events.

Closes cockpit-project#7375

@stefwalter stefwalter force-pushed the stefwalter:shell-broken-ie branch from ac2a770 to cf63241 Jul 31, 2017

stefwalter added a commit to stefwalter/cockpit that referenced this pull request Jul 31, 2017

shell: Fix shell exceptions on Internet Explorer
Simply clicking from one screen to another in Internet Explorer
results in exceptions and Ooops being displayed. This is due
to differences in behavior on what is acessible and which
properties are available on certain events.

Closes cockpit-project#7375

@stefwalter stefwalter force-pushed the stefwalter:shell-broken-ie branch from 58f97e8 to 410153c Aug 1, 2017

stefwalter added a commit to stefwalter/cockpit that referenced this pull request Aug 1, 2017

shell: Fix shell exceptions on Internet Explorer
Simply clicking from one screen to another in Internet Explorer
results in exceptions and Ooops being displayed. This is due
to differences in behavior on what is acessible and which
properties are available on certain events.

Closes cockpit-project#7375

@stefwalter stefwalter force-pushed the stefwalter:shell-broken-ie branch 3 times, most recently from ef55936 to ffb1b77 Aug 1, 2017

stefwalter added a commit to stefwalter/cockpit that referenced this pull request Aug 2, 2017

shell: Fix shell exceptions on Internet Explorer
Simply clicking from one screen to another in Internet Explorer
results in exceptions and Ooops being displayed. This is due
to differences in behavior on what is acessible and which
properties are available on certain events.

Closes cockpit-project#7375
@stefwalter

This comment has been minimized.

Copy link
Contributor Author

commented Aug 2, 2017

Fixed all the basics.

@martinpitt
Copy link
Member

left a comment

Looks mostly good, just one question/improvement

try {
source = source_by_name[child.name];
} catch(ex) {
return;

This comment has been minimized.

Copy link
@martinpitt

martinpitt Aug 2, 2017

Member

Blanket catches always smell dubious, as they are prone to silently ignoring real/unexpected exceptions. How does such an ex for an Access Denied look like? Is there anything predictable in it which this catch could use to restrict this? Could this at least console.warn(ex) for debugging?

This comment has been minimized.

Copy link
@stefwalter

stefwalter Aug 2, 2017

Author Contributor

How does such an ex for an Access Denied look like? Is there anything predictable in it which this catch could use to restrict this?

This is really hard to do in a cross browser fashion. Javascript doesn't help us here at all, so we're limited to screen scraping, which is crazy in browser exceptions.

Could this at least console.warn(ex) for debugging?

Sure. But it's not really a warning. It's us processing data from a window that no longer exists, we really should just ignore the window.

This comment has been minimized.

Copy link
@martinpitt

martinpitt Aug 2, 2017

Member

OK, thanks for the explanation! Fine for me then.

shell: Fix shell exceptions on Internet Explorer
Simply clicking from one screen to another in Internet Explorer
results in exceptions and Ooops being displayed. This is due
to differences in behavior on what is acessible and which
properties are available on certain events.

Closes #7375

@stefwalter stefwalter force-pushed the stefwalter:shell-broken-ie branch from ffb1b77 to 6474440 Aug 2, 2017

@martinpitt
Copy link
Member

left a comment

I see you already added the logging. Thanks!

@martinpitt martinpitt merged commit 203d9e8 into cockpit-project:master Aug 2, 2017

11 of 17 checks passed

verify/fedora-26 1 tests failed
Details
verify/rhel-7 1 tests failed
Details
verify/rhel-7-4 1 tests failed
Details
verify/rhel-atomic 1 tests failed
Details
verify/centos-7 Testing in progress [cockpit-tests-n97hx]
Details
verify/ubuntu-stable Testing in progress [cockpit-tests-5wk7p]
Details
avocado/fedora-24 Tests passed
Details
container/kubernetes Tests passed
Details
selenium/chrome Tests passed
Details
selenium/explorer Tests passed
Details
selenium/firefox Tests passed
Details
semaphoreci The build passed on Semaphore.
Details
verify/debian-stable Tests passed
Details
verify/debian-testing Tests passed
Details
verify/fedora-atomic Tests passed
Details
verify/fedora-i386 Tests passed
Details
verify/ubuntu-1604 Tests passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.