Make WordPress Core

Opened 11 years ago

Closed 9 years ago

#28322 closed defect (bug) (fixed)

wpview: focus problem in Chrome making impossible to edit a visual shortcode

Reported by: bduclos's profile bduclos Owned by:
Milestone: 4.6 Priority: normal
Severity: normal Version: 3.9
Component: TinyMCE Keywords: has-patch needs-testing
Focuses: javascript, administration Cc:

Description

To reproduce, create a gallery (with nothing before), add a very long text after it and create another gallery immediately after it.

If you try to select the second gallery to edit or remove it, it returns you at the top of the editor making it impossible to edit or remove the gallery.

This issue only happens in Chrome. Line 93: editor.getBody().focus();
in the wpview/plugin.js seems to be the issue. (Removing this line solves the problem)

Attachments (1)

28322.patch (2.7 KB) - added by azaozz 9 years ago.

Download all attachments as: .zip

Change History (22)

#1 @azaozz
11 years ago

  • Milestone Awaiting Review deleted
  • Resolution set to worksforme
  • Status changed from new to closed

Cannot reproduce this in trunk and Chrome 35.0.1916.153. Feel free to reopen if still a problem.

#2 @bduclos
11 years ago

  • Resolution worksforme deleted
  • Status changed from closed to reopened

Tested in trunk and Chrome 35.0.1916.153 on different computers (Windows 8, 7 and Vista), still the same problem.
Using the Twenty Fourteen theme, in Visual Mode when I click on the second gallery to edit it, the scrollbar of the editor returns at the top making it impossible to edit it.
Below is the content that I enter on the page:

[gallery ids="19,20"]

Lorem ipsum dolor sit amet, consectetur adipiscing elit. Phasellus consectetur, massa vel vulputate ullamcorper, nunc risus tincidunt eros, eu tincidunt dui erat quis metus. In consectetur sem felis, quis consectetur leo pellentesque ac. Cras pretium odio quis lobortis iaculis. Duis lectus dui, interdum eget dui non, mattis vestibulum ante. Quisque eros nibh, placerat non velit id, consectetur laoreet lorem. Sed congue justo leo, id pellentesque lacus tincidunt at. Mauris consequat quis justo sit amet tincidunt. Integer ac risus eget enim molestie feugiat quis quis nibh. Etiam sed dui orci. Phasellus massa metus, ornare et ipsum id, aliquet viverra nibh. Ut ipsum enim, malesuada at ante id, ornare rutrum mi. Etiam id lacus sodales, feugiat odio ut, feugiat quam. Nullam semper justo adipiscing aliquam blandit. Curabitur blandit lectus sit amet nisi placerat elementum.

Etiam accumsan, quam sit amet consectetur condimentum, massa mauris blandit ipsum, eget varius elit turpis eget mi. Class aptent taciti sociosqu ad litora torquent per conubia nostra, per inceptos himenaeos. Vestibulum dignissim leo quis erat tincidunt tincidunt sed eget ipsum. Aliquam eget erat lacus. Praesent rutrum augue vitae velit hendrerit, quis blandit erat semper. Ut non lacus quis diam eleifend lobortis et ut odio. Nulla ultricies metus non elit interdum, at condimentum nunc sagittis. Ut suscipit blandit vulputate. Proin malesuada risus magna, sit amet hendrerit massa mattis ac. Etiam felis est, tristique quis malesuada eget, ornare ac nunc.

Nam bibendum, ipsum ut fermentum laoreet, sapien sapien vestibulum libero, sit amet porttitor nisl sapien sit amet quam. Pellentesque ut nibh a velit posuere dignissim. In at ipsum eu leo blandit auctor. In hac habitasse platea dictumst. Integer et arcu augue. Vestibulum at mauris dignissim, viverra leo sit amet, tristique ante. Mauris tempor ipsum elit, id posuere nisl commodo a. Mauris tempor in leo rhoncus consectetur.

Aliquam accumsan eros eu iaculis dictum. Aenean hendrerit aliquam urna, id rutrum eros elementum quis. Praesent magna neque, ultricies sed tincidunt vel, rutrum dignissim odio. Vivamus lobortis aliquet risus, a iaculis urna cursus eu. Nunc id blandit sem, varius tincidunt erat. Vivamus feugiat ut leo quis rhoncus. Donec egestas vitae urna eget bibendum. Mauris vel interdum ligula, non pretium felis. Mauris id sapien sed mi sollicitudin tempus. Pellentesque at nisl in nisi ornare dignissim id eget eros. Maecenas lobortis sed ipsum id ultricies. Pellentesque habitant morbi tristique senectus et netus et malesuada fames ac turpis egestas. Vestibulum eget semper mauris. Cum sociis natoque penatibus et magnis dis parturient montes, nascetur ridiculus mus.

[gallery ids="21"]

#3 @SergeyBiryukov
10 years ago

  • Milestone set to Awaiting Review

#4 @iseulde
10 years ago

  • Milestone Awaiting Review deleted
  • Resolution set to worksforme
  • Status changed from reopened to closed
  • Version changed from 3.9.1 to 3.9

Hi bduclos,

Thanks for the report. wpviews have changed a lot since 3.9 + editor scrolling in 4.0 and I cannot reproduce the issue. If you're still experiencing this issue in 4.1, please don't hesitate to reopen this ticket.

#5 @isabisa
9 years ago

  • Resolution worksforme deleted
  • Status changed from closed to reopened

#6 @isabisa
9 years ago

#36780 was marked as a duplicate.

#7 @isabisa
9 years ago

  • Focuses administration added

I just noticed this same problem and after some troubleshooting, found this ticket. This only happens in Chrome —I'm currently running version 50.0.2661.94 (64-bit) — and commenting out line 102 editor.getBody().focus(); solves the problem for me too.

The select() function is called on mousedown mouseup click touchend events. It seems like in Chrome (if you haven't previously clicked to place the text cursor inside the editor) on mousedown, the window jumps to the top of the page and then if ( viewNode !== selected ) evaluates to true, triggering the problem function on line 102.

I've had trouble isolating what is making the window scroll to top on mousedown though.

#8 @swissspidy
9 years ago

  • Milestone set to Awaiting Review

#9 @adamsilverstein
9 years ago

I was able to easily reproduce this issue. I am in Chrome 51.0.2704.36 beta. I created a post with a gallery at the top and bottom. if i click on the bottom gallery first, without first focusing the editor, the click appears to fail and i am taken to the top of the page.

Screencast showing the issue:

http://cl.ly/442q3q0D452E/Screen%20Recording%202016-05-12%20at%2009.56%20AM.gif

#10 @azaozz
9 years ago

Hi @isabisa, thanks for closing the duplicate ticket and for finding where this is breaking.

Can reproduce too. Happens only in (new) Chrome. Seems to be a change of behaviour there. Also happens when the top wpView is selected and the user clicks on the bottom one. The gallery previews are non-editable "blocks" inside the contentEditable iframe body. Seems Chrome changed the way the focus "flows" between editable and non-editable elements.

Welcome to the wondrous world of contentEditable! :)

That particular line comes from the original code of wpView, in WordPress 3.9. Think it was a workaround for a selection inconsistency at the time and is not needed any more, after all the changes to the surrounding code. Removing it will need more testing in all possible browsers, from IE9+ :)

#11 @adamsilverstein
9 years ago

@azaozz thanks for confirming, your assistance here is greatly appreciated!

I did some more testing but didn’t find a good solution. it seems like Chrome is ignoring the call to select the element we create for the view - the element is there, and targeted correctly, but the call to focus fails. In addition, the initial call to focus the editor scrolls to the top of the editor - this doesn't happen in firefox.

I will do more testing with that line removed across browsers via BrowserStack.

@azaozz
9 years ago

#12 @azaozz
9 years ago

  • Keywords has-patch needs-testing added
  • Milestone changed from Awaiting Review to 4.6

Seems the scrolling in recent Chrome versions is triggered by "overzealous" scroll-into-view.
28322.patch fixes that and some of the other problems. It also converts that bit from using editor.dom to editor.$.

However there are more problems in recent Chrome, especially when the "Enable full-height editor and distraction-free functionality." is turned off in Screen Options. Seems we will need another ticket for these.

#13 @iseulde
9 years ago

Can also reproduce with the current implementation. Tested as part of #36434, and it also solves the issue.

#14 @iseulde
9 years ago

Should be fixed in [37446], please test! :) Leaving open until we close #36434.

#15 follow-up: @isabisa
9 years ago

Thanks all for the help! @iseulde I figured out how to test patches using the instructions here https://make.wordpress.org/core/handbook/testing/patch/ but don't know how to test the changeset you linked to. Can you point me in the right direction?

#16 in reply to: ↑ 15 @iseulde
9 years ago

Replying to isabisa:

Thanks all for the help! @iseulde I figured out how to test patches using the instructions here https://make.wordpress.org/core/handbook/testing/patch/ but don't know how to test the changeset you linked to. Can you point me in the right direction?

If you're testing trunk, then it's already in there. :) If not, take a look around here: https://make.wordpress.org/core/handbook/tutorials/installing-wordpress-locally/#zip-file-svn-or-git

#17 @swissspidy
9 years ago

Another easy way to install the latest WordPress nightly build is by using the WordPress Beta Tester plugin.

#18 @iseulde
9 years ago

Works too. :) Not sure how often that's released.

#19 @isabisa
9 years ago

Thanks you guys! I was able to re-provision VVV and get it working. And the fix from @iseulde works for me!

This ticket was mentioned in Slack in #core-editor by iseulde. View the logs.


9 years ago

#21 @iseulde
9 years ago

  • Resolution set to fixed
  • Status changed from reopened to closed
Note: See TracTickets for help on using tickets.