Last modified: 2013-11-19 06:21:04 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 T34617, the corresponding Phabricator task for complete and up-to-date bug report information.
Bug 32617 - Incorrect edit summary preloading when editing section 0
Incorrect edit summary preloading when editing section 0
Product: MediaWiki
Classification: Unclassified
Interface (Other open bugs)
All All
: Low normal (vote)
: ---
Assigned To: Brion Vibber
Depends on:
Blocks: section-editing
  Show dependency treegraph
Reported: 2011-11-23 23:58 UTC by Jelle Zijlstra
Modified: 2013-11-19 06:21 UTC (History)
5 users (show)

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

Patch that changes the regex mode. (614 bytes, patch)
2011-11-25 00:46 UTC, Dan Collins

Description Jelle Zijlstra 2011-11-23 23:58:11 UTC
When editing section 0, the section in the edit summary automatically gets filled in when the section contains some sort of header code. Example (thanks to Anomie): .

This bug is apparently in the MediaWiki core. I think it can be fixed by adding a check for section 0 at line 1720 of includes/EditPage.php (change line 1720 to "if ( $this->section != '' && $this->section != 'new' && $this->section != '0' ) {". Since section 0 is the lead of the article, it'll never have a legitimate section heading, so this addition should not cause any problems.
Comment 1 Dan Collins 2011-11-25 00:46:28 UTC
Created attachment 9552 [details]
Patch that changes the regex mode.

The problem isn't necessarily only present in section zero - rather whenever there is a malformed section link. Since we're editing a section, we only care about section headers at the beginning of the current text we're editing, so I've changed the regex that looks for them from /m to /s, which causes the ^ to match the beginning of the string and not simply the beginning of the line. Patch is attached, causes no new unit test failures.
Comment 2 Jelle Zijlstra 2011-11-25 14:22:46 UTC
I don't think there will ever be a problem with any other section than section 0, because the first text in a section other than section 0 will always be the (correct) section header. Example:
Your patch would fail on an article that starts with an invalid section header, like "== foo == a".
Comment 3 Brion Vibber 2011-12-06 22:49:44 UTC
The regex change feels wrong to me; using dotall mode means that it may actually extend beyond finding your section title down into the text somewhere, if "=" or "==" or "===" etc appears somewhere later down in the text.

Perhaps remove both 'm' and 's' modifiers: make it match on the first line only, and don't attempt to go beyond the first line.
Comment 4 Brion Vibber 2011-12-06 22:51:46 UTC
Adding need-unittest keyword; this is a great candidate for lifting the text extraction out to a function and running some test text through it.
Comment 5 Brion Vibber 2011-12-06 23:37:22 UTC
r105379 extracts the function out and adds test cases.

I can confirm that applying the patch posted above fixes the "bogus section header in section 0" case, but creates the problem I describe in comment 3 when you have that happen in another section which has its own header with a matching "=" count.
Comment 6 Brion Vibber 2011-12-06 23:41:49 UTC
r105380 adds a modified fix which passes my test cases.

This removes both 'm' and 's' modifiers, and checks to make sure we're at the end of the line. It's possible that this breaks in some other legit cases.... but I think it's right in theory. :)

In theory. :P
Comment 7 Jelle Zijlstra 2011-12-07 02:58:07 UTC
That won't work; you can have comments at the end of a line containing a section header, after all. What's wrong with the suggestion I gave?
Comment 8 Brion Vibber 2011-12-07 03:01:47 UTC
Durrrr, how'd I skip over that? :P Yes, that should do the job, will poke at it later.
Comment 9 Sumana Harihareswara 2012-07-20 18:04:17 UTC
Is this problem still reproducible?  I don't think I've run into it.
Comment 10 Andre Klapper 2013-07-24 12:45:00 UTC
Patch superseded by code change in comment 6, removing keyword.
Comment 11 Krinkle 2013-11-19 06:20:50 UTC
I can't reproduce this bug anymore, works for me.

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