Skip to content

Commit

Permalink
Backport PR #5459: Fix interact animation page jump FF
Browse files Browse the repository at this point in the history
Firefox doesn't render images immediately as the data is available.  When animating the way that we animate, this causes the output area to collapse quickly before returning to its original size.  When the output area collapses, FireFox scrolls upwards in attempt to compensate for the lost vertical content (so it looks like you are on the same spot in the page, with respect to the contents below the image's prior location).  The solution is to resize the image output after the `img onload` event has fired.

This PR:
- Releases the `clear_output` height lock after the image has been loaded (instead of immediately or using a timeout).
- Removes a `setTimeout` call in the `append_output` method.
- `clear_output` in zmqshell no longer sends `\r` to the stream outputs.

closes #5128
  • Loading branch information
minrk committed Apr 9, 2014
1 parent 28baa39 commit ff1462d
Show file tree
Hide file tree
Showing 6 changed files with 111 additions and 36 deletions.
78 changes: 54 additions & 24 deletions IPython/html/static/notebook/js/outputarea.js
Original file line number Diff line number Diff line change
Expand Up @@ -268,39 +268,47 @@ var IPython = (function (IPython) {
});
return json;
};

OutputArea.prototype.append_output = function (json) {
this.expand();

// validate output data types
json = this.validate_output(json);

// Clear the output if clear is queued.
var needs_height_reset = false;
if (this.clear_queued) {
this.clear_output(false);
needs_height_reset = true;
}

// validate output data types
json = this.validate_output(json);

if (json.output_type === 'pyout') {
this.append_pyout(json);
} else if (json.output_type === 'pyerr') {
this.append_pyerr(json);
} else if (json.output_type === 'display_data') {
this.append_display_data(json);
} else if (json.output_type === 'stream') {
this.append_stream(json);
}

this.outputs.push(json);

// Only reset the height to automatic if the height is currently
// fixed (done by wait=True flag on clear_output).
if (needs_height_reset) {
this.element.height('');
}

// We must release the animation fixed height in a callback since Gecko
// (FireFox) doesn't render the image immediately as the data is
// available.
var that = this;
setTimeout(function(){that.element.trigger('resize');}, 100);
var handle_appended = function ($el) {
// Only reset the height to automatic if the height is currently
// fixed (done by wait=True flag on clear_output).
if (needs_height_reset) {
that.element.height('');
}
that.element.trigger('resize');
};
if (json.output_type === 'display_data') {
this.append_display_data(json, handle_appended);
} else {
handle_appended();
}

this.outputs.push(json);
};


Expand Down Expand Up @@ -475,9 +483,9 @@ var IPython = (function (IPython) {
};


OutputArea.prototype.append_display_data = function (json) {
OutputArea.prototype.append_display_data = function (json, handle_inserted) {
var toinsert = this.create_output_area();
if (this.append_mime_type(json, toinsert)) {
if (this.append_mime_type(json, toinsert, handle_inserted)) {
this._safe_append(toinsert);
// If we just output latex, typeset it.
if ((json['text/latex'] !== undefined) || (json['text/html'] !== undefined)) {
Expand All @@ -494,7 +502,7 @@ var IPython = (function (IPython) {
'image/jpeg' : true
};

OutputArea.prototype.append_mime_type = function (json, element) {
OutputArea.prototype.append_mime_type = function (json, element, handle_inserted) {
for (var type_i in OutputArea.display_order) {
var type = OutputArea.display_order[type_i];
var append = OutputArea.append_map[type];
Expand All @@ -511,7 +519,14 @@ var IPython = (function (IPython) {
}
}
var md = json.metadata || {};
var toinsert = append.apply(this, [value, md, element]);
var toinsert = append.apply(this, [value, md, element, handle_inserted]);
// Since only the png and jpeg mime types call the inserted
// callback, if the mime type is something other we must call the
// inserted callback only when the element is actually inserted
// into the DOM. Use a timeout of 0 to do this.
if (['image/png', 'image/jpeg'].indexOf(type) < 0 && handle_inserted !== undefined) {
setTimeout(handle_inserted, 0);
}
$([IPython.events]).trigger('output_appended.OutputArea', [type, value, md, toinsert]);
return toinsert;
}
Expand Down Expand Up @@ -611,10 +626,16 @@ var IPython = (function (IPython) {
if (width !== undefined) img.attr('width', width);
};

var append_png = function (png, md, element) {
var append_png = function (png, md, element, handle_inserted) {
var type = 'image/png';
var toinsert = this.create_output_subarea(md, "output_png", type);
var img = $("<img/>").attr('src','data:image/png;base64,'+png);
var img = $("<img/>");
if (handle_inserted !== undefined) {
img.on('load', function(){
handle_inserted(img);
});
}
img[0].src = 'data:image/png;base64,'+ png;
set_width_height(img, md, 'image/png');
this._dblclick_to_reset_size(img);
toinsert.append(img);
Expand All @@ -623,10 +644,16 @@ var IPython = (function (IPython) {
};


var append_jpeg = function (jpeg, md, element) {
var append_jpeg = function (jpeg, md, element, handle_inserted) {
var type = 'image/jpeg';
var toinsert = this.create_output_subarea(md, "output_jpeg", type);
var img = $("<img/>").attr('src','data:image/jpeg;base64,'+jpeg);
var img = $("<img/>");
if (handle_inserted !== undefined) {
img.on('load', function(){
handle_inserted(img);
});
}
img[0].src = 'data:image/jpeg;base64,'+ jpeg;
set_width_height(img, md, 'image/jpeg');
this._dblclick_to_reset_size(img);
toinsert.append(img);
Expand Down Expand Up @@ -748,7 +775,10 @@ var IPython = (function (IPython) {
this.clear_queued = false;
}

// clear all, no need for logic
// Clear all
// Remove load event handlers from img tags because we don't want
// them to fire if the image is never added to the page.
this.element.find('img').off('load');
this.element.html("");
this.outputs = [];
this.trusted = true;
Expand Down
4 changes: 0 additions & 4 deletions IPython/kernel/zmq/zmqshell.py
Original file line number Diff line number Diff line change
Expand Up @@ -90,11 +90,7 @@ def publish(self, source, data, metadata=None):

def clear_output(self, wait=False):
content = dict(wait=wait)

print('\r', file=sys.stdout, end='')
print('\r', file=sys.stderr, end='')
self._flush_streams()

self.session.send(
self.pub_socket, u'clear_output', content,
parent=self.parent_header, ident=self.topic,
Expand Down
44 changes: 37 additions & 7 deletions IPython/qt/console/frontend_widget.py
Original file line number Diff line number Diff line change
Expand Up @@ -197,6 +197,9 @@ def __init__(self, *args, **kw):
self._local_kernel = kw.get('local_kernel',
FrontendWidget._local_kernel)

# Whether or not a clear_output call is pending new output.
self._pending_clearoutput = False

#---------------------------------------------------------------------------
# 'ConsoleWidget' public interface
#---------------------------------------------------------------------------
Expand Down Expand Up @@ -339,6 +342,14 @@ def _insert_continuation_prompt(self, cursor):
#---------------------------------------------------------------------------
# 'BaseFrontendMixin' abstract interface
#---------------------------------------------------------------------------
def _handle_clear_output(self, msg):
"""Handle clear output messages."""
if not self._hidden and self._is_from_this_session(msg):
wait = msg['content'].get('wait', True)
if wait:
self._pending_clearoutput = True
else:
self.clear_output()

def _handle_complete_reply(self, rep):
""" Handle replies for tab completion.
Expand Down Expand Up @@ -520,6 +531,7 @@ def _handle_pyout(self, msg):
"""
self.log.debug("pyout: %s", msg.get('content', ''))
if not self._hidden and self._is_from_this_session(msg):
self.flush_clearoutput()
text = msg['content']['data']
self._append_plain_text(text + '\n', before_prompt=True)

Expand All @@ -528,13 +540,8 @@ def _handle_stream(self, msg):
"""
self.log.debug("stream: %s", msg.get('content', ''))
if not self._hidden and self._is_from_this_session(msg):
# Most consoles treat tabs as being 8 space characters. Convert tabs
# to spaces so that output looks as expected regardless of this
# widget's tab width.
text = msg['content']['data'].expandtabs(8)

self._append_plain_text(text, before_prompt=True)
self._control.moveCursor(QtGui.QTextCursor.End)
self.flush_clearoutput()
self.append_stream(msg['content']['data'])

def _handle_shutdown_reply(self, msg):
""" Handle shutdown signal, only if from other console.
Expand Down Expand Up @@ -685,6 +692,29 @@ def restart_kernel(self, message, now=False):
before_prompt=True
)

def append_stream(self, text):
"""Appends text to the output stream."""
# Most consoles treat tabs as being 8 space characters. Convert tabs
# to spaces so that output looks as expected regardless of this
# widget's tab width.
text = text.expandtabs(8)
self._append_plain_text(text, before_prompt=True)
self._control.moveCursor(QtGui.QTextCursor.End)

def flush_clearoutput(self):
"""If a clearoutput is pending, execute it."""
if self._pending_clearoutput:
self._pending_clearoutput = False
self.clear_output()

def clear_output(self):
"""Clears the current line of output."""
cursor = self._control.textCursor()
cursor.beginEditBlock()
cursor.movePosition(cursor.StartOfLine, cursor.KeepAnchor)
cursor.insertText('')
cursor.endEditBlock()

#---------------------------------------------------------------------------
# 'FrontendWidget' protected interface
#---------------------------------------------------------------------------
Expand Down
3 changes: 2 additions & 1 deletion IPython/qt/console/ipython_widget.py
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,6 @@ def __init__(self, *args, **kw):
#---------------------------------------------------------------------------
# 'BaseFrontendMixin' abstract interface
#---------------------------------------------------------------------------

def _handle_complete_reply(self, rep):
""" Reimplemented to support IPython's improved completion machinery.
"""
Expand Down Expand Up @@ -223,6 +222,7 @@ def _handle_pyout(self, msg):
"""
self.log.debug("pyout: %s", msg.get('content', ''))
if not self._hidden and self._is_from_this_session(msg):
self.flush_clearoutput()
content = msg['content']
prompt_number = content.get('execution_count', 0)
data = content['data']
Expand Down Expand Up @@ -250,6 +250,7 @@ def _handle_display_data(self, msg):
# eventually will as this allows all frontends to monitor the display
# data. But we need to figure out how to handle this in the GUI.
if not self._hidden and self._is_from_this_session(msg):
self.flush_clearoutput()
source = msg['content']['source']
data = msg['content']['data']
metadata = msg['content']['metadata']
Expand Down
2 changes: 2 additions & 0 deletions IPython/qt/console/rich_ipython_widget.py
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,7 @@ def _handle_pyout(self, msg):
""" Overridden to handle rich data types, like SVG.
"""
if not self._hidden and self._is_from_this_session(msg):
self.flush_clearoutput()
content = msg['content']
prompt_number = content.get('execution_count', 0)
data = content['data']
Expand All @@ -140,6 +141,7 @@ def _handle_display_data(self, msg):
""" Overridden to handle rich data types, like SVG.
"""
if not self._hidden and self._is_from_this_session(msg):
self.flush_clearoutput()
source = msg['content']['source']
data = msg['content']['data']
metadata = msg['content']['metadata']
Expand Down
16 changes: 16 additions & 0 deletions IPython/terminal/console/interactiveshell.py
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ class ZMQTerminalInteractiveShell(TerminalInteractiveShell):
"""A subclass of TerminalInteractiveShell that uses the 0MQ kernel"""
_executing = False
_execution_state = Unicode('')
_pending_clearoutput = False
kernel_timeout = Float(60, config=True,
help="""Timeout for giving up on a kernel (in seconds).
Expand Down Expand Up @@ -241,13 +242,22 @@ def handle_iopub(self, msg_id=''):
self._execution_state = sub_msg["content"]["execution_state"]
elif msg_type == 'stream':
if sub_msg["content"]["name"] == "stdout":
if self._pending_clearoutput:
print("\r", file=io.stdout, end="")
self._pending_clearoutput = False
print(sub_msg["content"]["data"], file=io.stdout, end="")
io.stdout.flush()
elif sub_msg["content"]["name"] == "stderr" :
if self._pending_clearoutput:
print("\r", file=io.stderr, end="")
self._pending_clearoutput = False
print(sub_msg["content"]["data"], file=io.stderr, end="")
io.stderr.flush()

elif msg_type == 'pyout':
if self._pending_clearoutput:
print("\r", file=io.stdout, end="")
self._pending_clearoutput = False
self.execution_count = int(sub_msg["content"]["execution_count"])
format_dict = sub_msg["content"]["data"]
self.handle_rich_data(format_dict)
Expand All @@ -267,6 +277,12 @@ def handle_iopub(self, msg_id=''):
if 'text/plain' in data:
print(data['text/plain'])

elif msg_type == 'clear_output':
if sub_msg["content"]["wait"]:
self._pending_clearoutput = True
else:
print("\r", file=io.stdout, end="")

_imagemime = {
'image/png': 'png',
'image/jpeg': 'jpeg',
Expand Down

0 comments on commit ff1462d

Please sign in to comment.