Last modified: 2014-09-02 16:01:52 UTC

Wikimedia Bugzilla is closed!

Wikimedia migrated from Bugzilla to Phabricator. Bug reports are handled in Wikimedia Phabricator.
This static website is read-only and for historical purposes. It is not possible to log in and except for displaying bug reports and their history, links might be broken. See T71447, the corresponding Phabricator task for complete and up-to-date bug report information.
Bug 69447 - [regression] Unable to get old toolbar in namespace Page: whatsoever the preference request for the old toolbar and break the wikieditor toolbar
[regression] Unable to get old toolbar in namespace Page: whatsoever the pref...
Status: VERIFIED FIXED
Product: MediaWiki extensions
Classification: Unclassified
ProofreadPage (Other open bugs)
unspecified
All All
: High major (vote)
: ---
Assigned To: Nobody - You can work on this!
https://en.wikisource.org/w/index.php...
: code-update-regression
Depends on:
Blocks: edit-toolbar 29908 Wikisource
  Show dependency treegraph
 
Reported: 2014-08-12 22:52 UTC by Philippe Elie
Modified: 2014-09-02 16:01 UTC (History)
11 users (show)

See Also:
Web browser: ---
Mobile Platform: ---
Assignee Huggle Beta Tester: ---


Attachments

Description Philippe Elie 2014-08-12 22:52:32 UTC
With preferences --> Edit --> deselect "Enable enhanced editing toolbar" we should get the old toolbar, this work in all namespace except in Page: namespace. Bug classified as Mediawiki extensions/ProofreadPage as I guess it comes from the javascript. Tpt if it doesn't come from the extension can you reclassify it please.
Comment 1 Umherirrender 2014-08-13 17:21:17 UTC
Maybe related to Gerrit change #141293 because the content model is not wikitext for that page.
Comment 2 Tpt 2014-08-13 19:48:40 UTC
The root cause of the bug is that now MediaWiki core only displays the old edit toolbar in pages with CONTENT_MODEL_WIKITEXT.

I believe that it's ProofreadPage that should display itself the toolbar so I keep this bug in ProofreadPage.
Comment 3 Derk-Jan Hartman 2014-08-13 19:51:59 UTC
eh those pages are not wikitext ?

what are they then ?
Comment 4 Tpt 2014-08-13 20:07:25 UTC
They have their own content model that is mostly the composition of 3 Wikitext areas (header, body and footer) with some data about the proofreading level of the text. Currently the storage serialization is still wikitext but it may be changed to a cleaner format like JSON. See the implementation of the PagePageContent: https://git.wikimedia.org/blob/mediawiki%2Fextensions%2FProofreadPage.git/1c5685425ba4bc41c174552d5e61b1d4de343043/includes%2Fpage%2FPageContent.php
Comment 5 Helder 2014-08-23 23:13:39 UTC
Change-Id: Ifafe41f7bc91183e0db4c10d8340a8b620b380bc.
Comment 6 Gerrit Notification Bot 2014-08-26 02:49:58 UTC
Change 156009 had a related patch set uploaded by Legoktm:
Revert "Toolbar: Only show on WikiText pages"

https://gerrit.wikimedia.org/r/156009
Comment 7 Gerrit Notification Bot 2014-08-29 10:27:15 UTC
Change 157054 had a related patch set uploaded by Tpt:
Adds a method to EditPage that allows extensions to show/hide the toolbar

https://gerrit.wikimedia.org/r/157054
Comment 8 Philippe Elie 2014-08-31 11:39:37 UTC
Tpt, there should be a second patch depending on this one to overload this new function in the proofreadpage code ?

I increase priority, I didn't notice it but it's not only the old toolbar which is broken but the enhanced does not behave correctly too. (inserting an internal link does not open a popup window but insert [[Link title]]).
Comment 9 Andre Klapper 2014-08-31 13:03:19 UTC
Not "critical". See [[mw:Bugzilla/Fields#Severity]]
Comment 10 George Orwell III 2014-08-31 13:37:37 UTC
(In reply to Philippe Elie from comment #8)
> I increase priority, I didn't notice it but it's not only the old toolbar
> which is broken but the enhanced does not behave correctly too. (inserting
> an internal link does not open a popup window but insert [[Link title]]).

Fwiw... the pop-up windows (known as 'dialogs' in the WikiEditor extension) only work if both 'Enable enhanced editing toolbar' and 'Enable wizards for inserting links, tables as well as the search and replace function' are enabled in your User preferences. What you described is normal for when ONLY 'Enable enhanced editing toolbar' is selected.

Since the change that caused this bug happened, disabling 'Show edit toolbar' in your User preferences has help some Users: get over similar issues even though that option should have nothing to with the enhanced toolbar (WikiEditor)
if both are enabled [allegedly, that is - others say disabling it should disable both the classic and WikiEditor toolbars while that is obviously not the case, if it ever was]

That said - can we get some movement on this or what? The quirkiness being discovered only grows with every passing day now.
Comment 11 Philippe Elie 2014-08-31 13:42:37 UTC
I enabled both of them but I can't get the wizard to work, is it working for you ?
Comment 12 Tpt 2014-08-31 13:54:08 UTC
(In reply to Philippe Elie from comment #8)
> Tpt, there should be a second patch depending on this one to overload this
> new function in the proofreadpage code ?
Not yet as I wait the core change to be merged. The ProofreadPage change will very small but I would like to see the core change to be reviewed first in order to don't have to change the Prp change twice or more because of changes in the core change done during the review process.
Comment 13 George Orwell III 2014-08-31 13:55:15 UTC
(In reply to Philippe Elie from comment #11)
> I enabled both of them but I can't get the wizard to work, is it working for
> you ?

Yes, the dialogs work for me when I have both the enhanced & wizards enabled and 'show edit toolbar' disabled. I typically don't use the wizards so I will disable them again after I post this.

When I have all three enabled, dialogs still work for me but tangent items like the CharacterInsert extension (via a local gadget) and similar customizations start "breaking down" and are no longer consistently generated nor reliable when selected/executed.
Comment 14 George Orwell III 2014-08-31 14:15:02 UTC
(In addition to George Orwell III from comment #13)
> When I have all three enabled, dialogs still work for me but tangent items
> like the CharacterInsert extension (via a local gadget) and similar
> customizations start "breaking down" and are no longer consistently
> generated nor reliable when selected/executed.

bug 70233 is similar to what I meant by "breaking down" along with several other User:s on en.WS who have reported much the same.
Comment 15 Bartosz Dziewoński 2014-09-01 12:19:06 UTC
There are so many pending conflicting patches and related bugs, I am lost. Can somebody summarize the proposed solutions and their corresponding patch groups, and preferably also say which one is the best and should be merged?
Comment 16 Helder 2014-09-01 13:03:30 UTC
One week ago I suggested the original patch to be reverted (as a "quick" fix)
https://gerrit.wikimedia.org/r/156009
but it seems it'll get as much time as any other patch to get merged...
Comment 17 Gerrit Notification Bot 2014-09-01 13:50:11 UTC
Change 157679 had a related patch set uploaded by Bartosz Dziewoński:
Revert "Toolbar: Only show on WikiText pages"

https://gerrit.wikimedia.org/r/157679
Comment 18 Gerrit Notification Bot 2014-09-01 13:50:45 UTC
Change 157680 had a related patch set uploaded by Bartosz Dziewoński:
Revert "Toolbar: Only show on WikiText pages"

https://gerrit.wikimedia.org/r/157680
Comment 19 Bartosz Dziewoński 2014-09-01 13:54:02 UTC
Should have poked someone about it. I +2'd the revert (jenkins seems stuck though) and scheduled it from SWAT deployment today in about an hour (https://wikitech.wikimedia.org/wiki/Deployments#Near-term).

Now, can somebody summarize the proposed solutions and their corresponding patch groups, and preferably also say which one is the best and should be merged? :D
Comment 20 Gerrit Notification Bot 2014-09-01 14:07:09 UTC
Change 156009 merged by jenkins-bot:
Revert "Toolbar: Only show on WikiText pages"

https://gerrit.wikimedia.org/r/156009
Comment 21 Helder 2014-09-01 14:21:58 UTC
I believe there are only two proposed solutions:
1) Create a function shouldShowToolbar which returns true if the content model is wikitext (extensions would subclass EditPage I think):
https://gerrit.wikimedia.org/r/#/c/157054/1/includes/EditPage.php,unified

2) Create a hook to allow extensions to redefined the value of the variable $showToolbar:
https://gerrit.wikimedia.org/r/#/c/120357/6/includes/EditPage.php,unified
Comment 22 George Orwell III 2014-09-01 21:50:48 UTC
Forgive my "newbie-ness" here, I know 'soap-box' like speeches are frowned up here, but if...

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

... is indeed the reversion of the change(s) that caused the problem reported in this bug in the first place, what else is there to actually "solve" here?

Maybe I reached this conclusion because I'm in the same boat as the comments made back in #c15 - I can't keep the needed/wanted regressions separate from the pseudo-patches and/or semi-enhancements that all seem to be lumped together for no discernable good reason now.

Granted, my participation here is rather recent but I assure you I've been lurking here for some time now and my observation has always been that there are too many conflicting interests at play all at once. Eventually these interests seem to 'muddy the waters' of a particular bugzilla by arbitrarily usurping some reports as duplicates without proper rationale, diffuse the focus of report by peppering the "see also" list with tangent issues made in other reports or build little self-serving cabals by making other reports blocking/dependent on what should be stand-alone issues or concerns.

That said, if the above linked merge resolves the reported issue(s) then I'm of the opinion that this bug should be closed and any further changes should be taken up in the appropriate bugzillas (if they exist) or open a new one addressing whatever needs resolving/discussion.
Comment 23 Bartosz Dziewoński 2014-09-01 22:12:58 UTC
You're right, the revert fixes this bug, and we should discuss how to re-implement the thing that caused it on bug 29908 (without breaking things this time ;) ).

The revert hasn't been deployed on the affected wikis yet, I was confused when I wrote comment 19. It will actually be deployed tomorrow. Nevertheless let's close this bug.
Comment 24 George Orwell III 2014-09-02 06:02:36 UTC
Errr.... possible stupid question but please humor my ignorance just in case its not:

Did we inadvertently remove some "stuff" from EditPage.php with the merging of...

https://gerrit.wikimedia.org/r/#/c/156009/3/includes/EditPage.php

... instead of merging the "more reflective" current state of the file - also containing the reversion of the troublemaking code - in ...

https://gerrit.wikimedia.org/r/#/c/157680/1/includes/EditPage.php

...?

I only ask because if you toggle between the two, you can clearly see the part dealing with "blankPages" right after the section we're dealing with in the reversion is present in #157680 (as well as in #157679 while we're at it), but not in #156009.

And, as I far as I can tell, the part dealing with "blankPages" is also present in the current .php file.
Comment 25 Gerrit Notification Bot 2014-09-02 15:02:51 UTC
Change 157679 merged by jenkins-bot:
Revert "Toolbar: Only show on WikiText pages"

https://gerrit.wikimedia.org/r/157679
Comment 26 Gerrit Notification Bot 2014-09-02 15:03:05 UTC
Change 157680 merged by jenkins-bot:
Revert "Toolbar: Only show on WikiText pages"

https://gerrit.wikimedia.org/r/157680
Comment 27 Bartosz Dziewoński 2014-09-02 15:26:22 UTC
The revert has been deployed now.

Re comment 24, that just means that the patches are based on a slightly different version of MediaWiki. That piece of code dealing with blank pages was added in change c3fcaba0, which wasn't yet merged when the original patch was written, but was already merged when it was backported to 1.24wmf19 for deployment. Nothing to worry about :)

Note You need to log in before you can comment on or make changes to this bug.


Navigation
Links