WordPress.org

Make WordPress Core

Opened 7 years ago

Closed 7 years ago

#27761 closed defect (bug) (fixed)

Quick Draft crashes in IE8

Reported by: SergeyBiryukov Owned by: nacin
Milestone: 3.9 Priority: high
Severity: critical Version: 3.9
Component: Administration Keywords: has-patch commit
Focuses: Cc:

Description

Once I click or tab to "What’s on your mind?" textarea in IE8, the browser tab crashes.

Only happens in trunk, could not reproduce in 3.8.2.

Attachments (6)

27761.diff (572 bytes) - added by morganestes 7 years ago.
dashboard.js
27761.2.diff (1.4 KB) - added by azaozz 7 years ago.
ie9.quickdraft.png (13.4 KB) - added by ocean90 7 years ago.
ie9.quickdraft-38.png (14.2 KB) - added by ocean90 7 years ago.
27761.3.diff (612 bytes) - added by azaozz 7 years ago.
27761.4.diff (887 bytes) - added by morganestes 7 years ago.

Download all attachments as: .zip

Change History (22)

#1 @nacin
7 years ago

  • Priority changed from normal to high
  • Severity changed from normal to critical

@morganestes
7 years ago

dashboard.js

#2 @morganestes
7 years ago

  • Keywords has-patch added

I was able to narrow this down to autoResizeTextarea() watching for onpropertychange to fire (along with focus and input). Having that in there consistently crashes IE8 on XP in my testing.

Two options are to either remove the call to autoResizeTextarea() inside quickPressLoad(), or to remove propertychange from the jQuery.on() call, which is what I opted to do here.

This change allows me to successfully create a Quick Draft in IE8 without any crashes, and this function is only used one time in this instance.

This ticket was mentioned in IRC in #wordpress-dev by morganestes. View the logs.


7 years ago

#4 @azaozz
7 years ago

Removing onpropertychange stops the auto-resize while typing in IE8. That's probably acceptable as long as there is a vertical scrollbar. In this case we should just disable the auto-resizing in IE < 9 and add overflow: auto.

Last edited 7 years ago by azaozz (previous) (diff)

#5 @azaozz
7 years ago

Actually not sure how this worked in the first place :) Looking at the docs, onpropertychange fires when an element's style property changes, but we are setting the style in the callback creating a loop.

#6 follow-up: @morganestes
7 years ago

Another thing I just came across is a bug in IE8 with overflow-y and max-height properties. I've tried several changes, including adjusting the 1300px max-height in CSS and changing the rules in the JS.

I've been able to get autosizing to work in the textarea without crashing the browser by adding back in propertychange to the .on function call and removing editor.css('overflow-y', 'auto');. The drawback to this is we lose the scrollbars.

@azaozz
7 years ago

#7 in reply to: ↑ 6 ; follow-up: @azaozz
7 years ago

Replying to morganestes:

Nice catch. Seems setting the overflow there always goes to infinite loop in IE8.

27761.2.diff solves it here, could you test/confirm?

#8 in reply to: ↑ 7 @morganestes
7 years ago

Replying to azaozz:

27761.2.diff solves it here, could you test/confirm?

It looks good on a read-through. I won't be available to test it out until Saturday afternoon, but I'll do so as soon as I'm able.

#9 @ocean90
7 years ago

27761.2.diff tested in Chrome 33, Opera 12, Firefox 28, IE 8, 10and 11, looks good.
IE 7 doesn't change the height, it just shows the vertical scrollbars, fine too.

IE 9 shows horizontal scrollbars after the default height is exceeded, with and without the patch. See ie9.quickdraft.png. This is a regression, see ie9.quickdraft-38.png.

#10 follow-up: @nacin
7 years ago

I really don't care if we just don't grow the textarea at all in IE < 9. Just bail and let it get a scrollbar. Sounds easier?

@azaozz
7 years ago

#11 in reply to: ↑ 10 @azaozz
7 years ago

Replying to nacin:

Either way works :) 27761.2.diff fixes the IE8 crash, 27761.3.diff removes the autoresize for IE < 9.

To fix IE9 scrollbars perhaps we can increase the size adjustment a bit:

cloneHeight = clone.css('width', $this.css('width')).text(textareaContent).outerHeight() + 2;

That + 2 at the end can probably be + 5 or even + 10. Don't have native IE9 at the moment to test with.

#12 @morganestes
7 years ago

How about a blend of patches?

Patch 4:

  • returns early for IE < 9
  • removes autoresize and keeps scrollbars in the textarea
  • prevents IE 8 from crashing;
  • allows textarea expanding in IE 9 with a vertical scrollbar, but hides the horizontal scrollbar.

@morganestes
7 years ago

#13 @nacin
7 years ago

Sounds good to me. Can I get confirmation from someone this is good to go?

#14 @ocean90
7 years ago

  • Keywords commit added

27761.4.diff has my Go.

#15 @azaozz
7 years ago

Seems to work well, still can't test in native IE9. Setting editor.css('overflow-y', 'auto'); and then editor.css('overflow', 'hidden'); produces a bit inconsistent results in different browsers, doesn't seem to break anything though.

#16 @nacin
7 years ago

  • Owner set to nacin
  • Resolution set to fixed
  • Status changed from new to closed

In 28103:

Don't auto-resize Quick Draft textarea in IE < 9. Avoids IE8 crash.

props morganestes.
fixes #27761.

Note: See TracTickets for help on using tickets.