New issue
Advanced search Search tips

Issue 855925 link

Starred by 1 user

Issue metadata

Status: Available
Owner: ----
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

hterm: creating a hterm.Terminal but not calling decorate() triggers: Uncaught TypeError: Cannot read property 'style' of undefined

Reported by zib...@gmail.com, Jun 24 2018

Issue description

UserAgent: Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/67.0.3396.87 Safari/537.36

Steps to reproduce the problem:
1. downloaded https://chromium.googlesource.com/apps/libapps/+archive/2056832f0b287924110849cecbf1e5019c1a51c3.tar.gz
2. executed bin/mkdist.sh
3. added hterm_all.js to index.html

What is the expected behavior?

What went wrong?
hterm.defaultStorage = new lib.Storage.Local();
lib.Storage.Local {observers_: Array(0), storage_: Storage}
term = new hterm.Terminal()
hterm.Terminal {profileId_: "default", primaryScreen_: h…m.Screen, alternateScreen_: h…m.Screen, screen_: h…m.Screen, screenSize: h…m.Size, …}
hterm_all.js:10884 Uncaught TypeError: Cannot read property 'style' of undefined
    at hterm.ScrollPort.setBackgroundColor (hterm_all.js:10884)
    at hterm.Terminal.setBackgroundColor (hterm_all.js:12613)
    at Array.background-color (hterm_all.js:12223)
    at hterm.PreferenceManager.lib.PreferenceManager.notifyChange_ (hterm_all.js:2422)
    at hterm.PreferenceManager.lib.PreferenceManager.notifyAll (hterm_all.js:2402)
    at hterm.Terminal.<anonymous> (hterm_all.js:12534)
hterm.ScrollPort.setBackgroundColor @ hterm_all.js:10884
hterm.Terminal.setBackgroundColor @ hterm_all.js:12613
background-color @ hterm_all.js:12223
lib.PreferenceManager.notifyChange_ @ hterm_all.js:2422
lib.PreferenceManager.notifyAll @ hterm_all.js:2402
(anonymous) @ hterm_all.js:12534
setTimeout (async)
(anonymous) @ hterm_all.js:2290
setTimeout (async)
lib.Storage.Local.getItems @ hterm_all.js:3276
lib.PreferenceManager.readStorage @ hterm_all.js:2271
hterm.Terminal.setProfile @ hterm_all.js:12533
hterm.Terminal @ hterm_all.js:12111
(anonymous) @ VM5416:1

Did this work before? N/A 

Chrome version: 67.0.3396.87  Channel: stable
OS Version: 
Flash Version:
 

Comment 1 Deleted

Comment 2 Deleted

Comment 3 by vapier@chromium.org, Jun 25 2018

Cc: raymes@chromium.org
Labels: -OS-Linux -Via-Wizard-Javascript -Needs-Triage-M67
Status: Available (was: Unconfirmed)
Summary: hterm: creating a hterm.Terminal but not calling decorate() triggers: Uncaught TypeError: Cannot read property 'style' of undefined (was: Uncaught TypeError: Cannot read property 'style' of undefined)
i'm guessing you're running this in a JS console.  if you call the decorate() method on an object, it should work.  i agree it shouldn't be crashing though even in this limited usage.

light triage suggests this is due to the new a11y code.  scrollUpButton_ isn't created until decorate is called, but the pref notification/sync steps might kick in before that and hit the scrollport.setBackgroundColor code paths:
hterm.ScrollPort.prototype.setBackgroundColor = function(color) {
  this.screen_.style.backgroundColor = color;
  this.scrollUpButton_.style.color = color;    <-- doesn't exist
  this.scrollDownButton_.style.color = color;
};

Comment 4 by vapier@chromium.org, Jun 25 2018

my suggestion for fixing this would be to convert the default background color to a CSS var.  we have a terminal.setCssVar helper already for it.

Comment 5 by raymes@chromium.org, Jun 25 2018

I'll take a look at this today

Comment 6 by raymes@chromium.org, Jun 26 2018

I don't think this is related to the new ally code. screen_ is also initialized in decorate and when I run the code in the original report, I get the exception on:
this.screen_.style.backgroundColor = color;

So yes, probably a different approach is necessary as you suggest, or else adding null checks. At the same time I think the issue occurs as a result of having a previously created terminal which may not be one that we want to support?

Comment 7 by vapier@chromium.org, Jun 26 2018

Cc: -raymes@chromium.org
Labels: -Pri-2 Pri-3
thanks for pointing that out.  you're right this isn't related to a11y.

poking at random old versions, this has been failing since at least hterm-1.40 (May 2014), so clearly not a regression or high priority :).

Sign in to add a comment