VisualEditor: <div class="ve-ce-protectedNode"> not always preventing editing in VE
Closed, DeclinedPublic

Description

Author: kwwilliams

Description:
When articles contain items that lead VE to corrupt the article, we've been protecting them by wrapping the article with <div class="ve-ce-protectedNode"> and </div>. This used to keep VE from being able to select any material within the article, which allowed us to keep the article from being corrupted.

VE is now allowing edits to templates inside the article (see https://en.wikipedia.org/w/index.php?title=Teenage_Dream_%28Katy_Perry_album%29&veaction=edit for an example), so we now have no way to protect articles that we know VE cannot handle.


Version: unspecified
Severity: enhancement
See Also:
https://bugzilla.wikimedia.org/show_bug.cgi?id=52141

bzimport added a project: VisualEditor-MediaWiki.Via ConduitNov 22 2014, 1:50 AM
bzimport set Reference to bz53767.
bzimport created this task.Via LegacySep 4 2013, 6:06 PM
Thryduulf added a comment.Via ConduitSep 4 2013, 6:19 PM

The original person to report this noted that they were using Chrome 29 on Windows 7.

I have been unable to reproduce this in Firefox 23 on Linux.

Neither kww or the ip who can reproduce this have noted their system.

bzimport added a comment.Via ConduitSep 4 2013, 6:24 PM

kwwilliams wrote:

Firefox 23.0.1 on Windows 7.

To be clear, we are still prevented from editing the text. We are not prevented from editing templates, and we can insert text before div.

Thryduulf added a comment.Via ConduitSep 4 2013, 6:53 PM

According to the documentation, the div is only meant to block editing to a section of a page (although that section can be the whole text), so adding text before is correct, likewise you should be able to add text after the /div.

The editing templates within the div does sound like a bug though.

If VE (or Parsoid possibly) is not treating the div as invariant then the div definitely isn't doing what it was intended to do.

bzimport added a comment.Via ConduitSep 4 2013, 7:02 PM

kwwilliams wrote:

(In reply to comment #3)

According to the documentation, the div is only meant to block editing to a
section of a page (although that section can be the whole text), so adding
text
before is correct, likewise you should be able to add text after the /div.

It's a change from the original behaviour. I agree that it's not technically wrong.

bzimport added a comment.Via ConduitSep 4 2013, 7:28 PM

kwwilliams wrote:

The div has been removed, so people need to look at https://en.wikipedia.org/w/index.php?title=Teenage_Dream_%28Katy_Perry_album%29&oldid=571540632

Jdforrester-WMF added a comment.Via ConduitSep 4 2013, 7:35 PM

(In reply to comment #5)

The div has been removed, so people need to look at
https://en.wikipedia.org/w/index.
php?title=Teenage_Dream_%28Katy_Perry_album%29&oldid=571540632

In this particular case, note that the reason everything was broken in editing this page was because the <ref> was defined outside of the table cell, in a the non-space between the end of the cell and the start of the row, and only happened to "work" in WT mode because of HTML Tidy moving it to somewhere sane - Ed's edit to the page fixed the duplication so the desired protection is no longer needed: https://en.wikipedia.org/w/index.php?title=Teenage_Dream_(Katy_Perry_album)&diff=571550633&oldid=571540632

On the wider point, it's not clear if there's an actual long-term need for a proper VE-prevention system (or if it's all solvable with a few quick fixes of bad wikitext), but if there is, we should build a proper one, not rely on a hack that might break.

bzimport added a comment.Via ConduitSep 4 2013, 7:55 PM

kwwilliams wrote:

(In reply to comment #6)

On the wider point, it's not clear if there's an actual long-term need for a
proper VE-prevention system (or if it's all solvable with a few quick fixes
of
bad wikitext), but if there is, we should build a proper one, not rely on a
hack that might break.

What's unclear about that?

I agree that it would be better to do this with a category, but the need for VE to recognise some flag that tells it not to edit a given article seems fairly obvious. As long as VE mutilates pages in response to inputs that behaved reasonably with the working software, we will require a mechanism to tell VE not to edit certain pages.

NicoV added a comment.Via ConduitSep 5 2013, 11:14 AM

I tried again on my test page on a different computer running Firefox 17.0.2 (ESR) under XP.

At first, it seemed that the protected node was working, but at some point (after clicking, double-clicking, typing, ...) I managed to write in the protected node: the result was an even greater mess than before: still the duplication, quotes added all over the place (they seem correct but unnecessary), pipes removed, space added, ...

https://en.wikipedia.org/w/index.php?title=User%3ANicoV%2FTest&diff=571628361&oldid=571542702

bzimport added a comment.Via ConduitSep 8 2013, 8:20 PM

kwwilliams wrote:

Note https://en.wikipedia.org/w/index.php?title=Wikipedia:VisualEditor/Feedback&diff=572094345&oldid=572076135 , where another 26,000 articles that VE mutilates have been identified.

We really need to have a method to shield articles from VE. It's critical to do, and beyond reason to expect that we will always modify articles to conform to VE's expectations.

jayvdb added a comment.Via ConduitOct 1 2013, 2:28 PM

I was very easily able to reproduce this problem; I dont have exact steps to reproduce, but with a few clicks and keypresses in the content area, all content was removed and the text I typed appeared in the body of the article. Undo appeared to work as expected, however 'Review your changes' did not return any results - it hung, but didnt lock up the browser. The toolbar is another way to override the protectedNode.

This template is also being used to create a 'pre' text area on
https://en.wikipedia.org/w/index.php?title=Morse_code&veaction=edit

Jdforrester-WMF added a comment.Via ConduitOct 9 2013, 6:08 PM

Changing this to "Provide a way to block VisualEditor from being active on a page"; I worry that this will lead to yet another magic word, but oh well.

jayvdb added a comment.Via ConduitNov 8 2013, 4:50 AM

James, that is bug 52141. This bug was about a specific problem with the existing code.

TTO added a comment.Via ConduitJun 22 2014, 5:50 AM

As it stands, this is a dupe of bug 52141. I've put back the original bug summary; it seems to me that WONTFIX is the appropriate resolution for this bug.

Jdforrester-WMF added a comment.Via ConduitJun 26 2014, 12:07 AM

(In reply to This, that and the other from comment #13)

As it stands, this is a dupe of bug 52141. I've put back the original bug
summary; it seems to me that WONTFIX is the appropriate resolution for this
bug.

Agreed.

Add Comment