Page MenuHomePhabricator

Editing comments in flow on mw.org does not work in IE 11
Closed, ResolvedPublic

Description

steps to reproduce

Click edit on any topic and try editing the text underneath the title.

actual results

  • It hides the comment box so you carn't edit, it also disabled the cancel button so I coulden press cancel.

Expected results

  • I should be able to edit the topic correctly including the buttons working properly.

Event Timeline

Paladox created this task.Jun 21 2016, 9:43 PM
Restricted Application added subscribers: Zppix, Aklapper. · View Herald TranscriptJun 21 2016, 9:43 PM

I can't reproduce this. Could you perhaps attach a screenshot or a video?

Paladox added a comment.EditedJun 21 2016, 9:47 PM

@Catrope hi,

before I click the edit button here is what it looks like

and after I click the edit button

Are there any errors in the JS console?

@Catrope yes,

XML5632: Only one root element is allowed.

Line: 3, Column 2

Dosent allow me to view what the actual line is.

SyntaxError

load.php (118,398)

'rowspan','colspan'];xmlDoc=new DOMParser().parseFromString(html,'text/xml');for(i=0,len=maskAttrs.length;i<len;i++){fromAttr=unmask?'data-ve-'

clicking to get to the line starts at xmlDoc so maybe that be the problem.

Paladox added a comment.EditedJun 21 2016, 10:53 PM

Maybe it is a visualeditor error.

Seems editing work's in Microsoft edge but not in internet explorer. Might have been caused by https://github.com/wikimedia/VisualEditor/commit/741fad983ed9e531ed32148b2bec9afe9b3d2730 but not really sure.

Probably. Could you try with ?debug=true and see if you get a more useful error?

@Catrope doing that dosent show any error's but stop's js from working.

Does this break in Firefox or Chrome, or just in IE?

Paladox added a comment.EditedJun 21 2016, 11:01 PM

@Catrope seems to break in internet explorer only. Microsoft Edge works.

I'm running windows 10 insider preview latest build from last week.

Internet Explorer 11.

Does regular editing with VisualEditor work in IE11?

@Catrope yes using regular editing with visualediting works. So maybe it is something in flow broken.

Paladox added a comment.EditedJun 22 2016, 10:16 AM

@Catrope Openning it here https://www.mediawiki.org/wiki/Project:Support_desk and clicking edit on a comment returns this error

Unable to get property 'model' of undefined or null reference

This is the line it points too

{model=$.data(step.node,'view').model;if(countedNodes.indexOf(model)!==-1){return false;}countedNodes.push(model);return true;}

Meaning clicking on the edit button for the comment not the main topic text box.

But the main topic text box dosent work but editing a comment returns that error but shows the box.

Catrope reopened this task as Open.Aug 29 2016, 9:16 PM

Dammit, I duped in the wrong direction AGAIN.

Paladox added a comment.EditedAug 29 2016, 9:28 PM

It seems we are getting a new error. (new error described in T144164)

Could the priority be risen to high priority since it makes it diffulculty in editing your comment in flow please?

Mattflaschen-WMF renamed this task from Editing comments in flow on mw.org does not work to Editing comments in flow on mw.org does not work in IE 11.Aug 30 2016, 4:00 AM

(1) Reproduced in IE11 Windows 8.1 (Note: add replies/editing comments in IE9 on Windows 7 breaks the page - https://www.mediawiki.org/wiki/Topic:Sf6zgc7plebtit4v. The dark screen will be displayed.

(2) The errors are the same as reported in T144164: VisualEditor on Flow fails to allow you to edit a comment on Internet Explorer 11.

XML5632: Only one root element is allowed. Line: 5, Column 2


(3) As a side note: Editing a Board description in VE displays the following errors (reported as T139972: [Regression wmf.17] "Uncaught TypeError: Cannot read property 'model' of undefined" on clicking 'Edit description'

Unable to get property 'model' of undefined or null reference
SCRIPT5022: SyntaxError -- when switching between editors
File: eval code (47), Line: 73, Column: 398

It shows this error

SCRIPT5007: Unable to get property 'model' of undefined or null reference
load.php (312,448)

in Microsoft edge

it is refereeing to

{model=$.data(step.node,'view').model;if(countedNodes.indexOf(model)!==-1){return false;}countedNodes.push(model);return true;}

In my testing, I get the Unable to get property 'model' of undefined or null reference error when trying to edit a comment, and the Only one root element is allowed error when trying to edit the board description. Creating new topics and comments works fine.

Unable to get property 'model' of undefined or null reference comes from ve.ce.Document.js line 198: model = $.data( step.node, 'view' ).model;. There's some very weird behavior going on there where the VE integration in Flow tries to focus a surface that somehow isn't attached yet, and the target div is empty. This is not specific to Chrome and happens in IE too, see T139972: [Regression wmf.17] "Uncaught TypeError: Cannot read property 'model' of undefined" on clicking 'Edit description'. VE ends up in a very strange state where the CE surface is detached from the UI surface, which is in turn detached from the target.

SyntaxError and Only one root element is allowed come from ve.utils.js line 1125: xmlDoc = new DOMParser().parseFromString( html, 'text/xml' );. It looks like html is something like <p>Foo</p><p>Bar</p>, so that's a legitimate complaint, and not specific to board descriptions but to multi-paragraph input. This seems to be because ve.parseXhtml() expects to be given a full HTML document, but Flow only gives it fragments. This is fine for HTML parsing but not for XML parsing, and we only do XML parsing in IE to work around bugs. This is easily fixed by wrapping the HTML with body tags.

Unable to get property 'model' of undefined or null reference comes from ve.ce.Document.js line 198: model = $.data( step.node, 'view' ).model;. There's some very weird behavior going on there where the VE integration in Flow tries to focus a surface that somehow isn't attached yet, and the target div is empty. This is not specific to Chrome and happens in IE too, see T139972: [Regression wmf.17] "Uncaught TypeError: Cannot read property 'model' of undefined" on clicking 'Edit description'. VE ends up in a very strange state where the CE surface is detached from the UI surface, which is in turn detached from the target.

This turns out to be due to a race condition in Flow. I put in a guard against this, but it's not working.

SyntaxError and Only one root element is allowed come from ve.utils.js line 1125: xmlDoc = new DOMParser().parseFromString( html, 'text/xml' );. It looks like html is something like <p>Foo</p><p>Bar</p>, so that's a legitimate complaint, and not specific to board descriptions but to multi-paragraph input. This seems to be because ve.parseXhtml() expects to be given a full HTML document, but Flow only gives it fragments. This is fine for HTML parsing but not for XML parsing, and we only do XML parsing in IE to work around bugs. This is easily fixed by wrapping the HTML with body tags.

The attached patch should fix this.

Change 307672 had a related patch set uploaded (by Catrope):
Pass full HTML documents into VE, not fragments

https://gerrit.wikimedia.org/r/307672

Unable to get property 'model' of undefined or null reference comes from ve.ce.Document.js line 198: model = $.data( step.node, 'view' ).model;. There's some very weird behavior going on there where the VE integration in Flow tries to focus a surface that somehow isn't attached yet, and the target div is empty. This is not specific to Chrome and happens in IE too, see T139972: [Regression wmf.17] "Uncaught TypeError: Cannot read property 'model' of undefined" on clicking 'Edit description'. VE ends up in a very strange state where the CE surface is detached from the UI surface, which is in turn detached from the target.

This turns out to be due to a race condition in Flow. I put in a guard against this, but it's not working.

I figured out the cause of this, see T139972: [Regression wmf.17] "Uncaught TypeError: Cannot read property 'model' of undefined" on clicking 'Edit description' for more detail.

Oh thanks :)

Change 307672 merged by jenkins-bot:
Pass full HTML documents into VE, not fragments

https://gerrit.wikimedia.org/r/307672

Change 307820 had a related patch set uploaded (by Paladox):
Pass full HTML documents into VE, not fragments

https://gerrit.wikimedia.org/r/307820

Change 307820 merged by jenkins-bot:
Pass full HTML documents into VE, not fragments

https://gerrit.wikimedia.org/r/307820

Mentioned in SAL [2016-09-02T00:00:43Z] <dereckson@tin> Synchronized php-1.28.0-wmf.17/extensions/Flow/modules/flow/ui/widgets/editor/editors/mw.flow.ui.VisualEditorWidget.js: Flow Fixes related to Visual Editor (T138356 and T139972) (duration: 00m 45s)

Paladox added a comment.EditedSep 2 2016, 12:05 AM

This now works :)

Thanks for fixing the problem @Catrope

Checked in betalabs and testwiki wmf.18 - no issues is found for IE 11 (also checked IE10 and 9)

jmatazzoni closed this task as Resolved.Oct 11 2016, 12:59 AM
jmatazzoni claimed this task.