Wed, Dec 4
Tue, Dec 3
In a rare move, I'm changing this bug from "Declined" to "Resolved"! We didn't talk much about the stated subject of this bug report here, but we did talk about it elsewhere, and I was convinced to remove "None" and simply make all the radiobutton options unselected for a mandatory field, which I did here: fd34992e34a2
I'm marking this as "Declined", and after some private discussion, I think Krabina is okay with that decision. The reason I'm declining this request is that, for pages whose approved revision is not their latest, the admin presumably should first look at the difference between the two before deciding whether to approve the latest revision - so the "approve" link should be on the diff page (where it is already), not on Special:ApprovedRevs.
I just updated Approved Revs to use the ArticleRevisionViewCustom hook for new-ish versions of MediaWiki. I plan to do the same thing for my (still-unreleased) diagrams extension.
I guess this was done...
Marking this as "Declined"... I hope that's okay.
This was fixed in 1dd2b204755e, about a month after this bug was reported.
This was done, though with the flag renamed to "--force".
Closing this; feel free to re-open...
@hashar - okay, thanks, that was very helpful. I knew about the changeover to ArticleRevisionViewCustom (I actually created the mediawiki.org page for that hook!), but I wasn't expecting that change to Jenkins behavior. Anyway, I just checked in a fix for this to Approved Revs.
Mon, Dec 2
@alex-mashin: I've been thinking about this patch. It seems well-constructed; I'm just not sure that it's a good idea to create this caching table automatically. As far as I know, most usages of External Data don't need it - there are wikis that don't use #get_web_data at all, and of the ones that do, I think most hit APIs where server downtime is not a big issue. That's my assumption anyway, just judging by the relatively few questions I've gotten about caching. But maybe you have a different perspective. What do you think?
Sun, Dec 1
By the way - I think it makes sense to split up this patch into at least two, maybe even three patches. There are a lot of changes there that are unrelated to the topic of the patch, which is using a stale cache - the biggest set are the ones where array() is changed into . That's a useful change, but it would be clearer if it were in a separate patch.
Tue, Nov 26
Mon, Nov 25
Yes, I'm talking about both of those comments. For the refactoring - it may be good, but, as I said, (a) it's outside the scope of this bug fix, and (b) I'm not sure that this kind of partial refactor is better than no refactor at all.
I decided to go with a more comprehensive solution, which displays an error, or otherwise handles things, in other cases when a list of forms is supposed to be displayed (like #forminput). Hopefully this solution makes sense...
Sun, Nov 24
Thank you for this patch! It looks great.
@Tombolano - thank you for this fix. Is this backward-compatible? Page Forms is still meant to support MediaWiki (and thus WikiEditor) versions going back to 1.28.
Fri, Nov 22
Oh, that's interesting - I never thought to check the W3C standard.
I don't understand what you mean by "if a problem is reported, fix it". What would the fix be?
Okay, I think I get it - it sounds like what you're talking about now is just leaving the radiobutton unselected, as opposed to having a "None" value. Which is sort of an aesthetic issue, unrelated to whether there's a default value, although it's still potentially an improvement.
@Nicolas_NALLET - that's a good idea, and I just checked in some code that is meant to do this - if there are 7 or more columns, little "x"s show up for each column header that let you remove it from the spreadsheet. It would be great if you tried the latest Page Forms code, and let me know what you think. I'm marking this as "Resolved", but feel free to re-open if there is a problem.
I'm confused about the first thing you wrote - "Even if we posit that you are correct". Correct that the default value should not be used when editing existing pages? Because that seems to be the entire underlying issue.
Thu, Nov 21
Oh, yes, that's true - there's no "restricted" parameter in the field tag. I got confused because the commit message for the patch talks about restricted fields.
Alright, thanks. I haven't run the import, but I think I see the issue. The problem comes about when:
I never thought about that case... maybe it would be better to show an error message, like "No forms have been defined on this wiki", so it's clearer what's going on?
Wed, Nov 20
Mon, Nov 18
@Filburt - sorry for the long delay, and thank you for the fix!
Marking this as "Declined", although of course I'm grateful for the new tests. If you still want to talk about the original topic, feel free to re-open this.
Sun, Nov 17
Great, thank you!! Unit tests such as these are a big step in bringing Page Forms to the next level.
Fri, Nov 15
Great - I'm looking forward to checking in the additional tests. Page Forms could really use more tests!
Well, it's true that "None" is not valid, but I believe (and hope!) that the JS prevents the form from being submitted if that invalid value is chosen.
Thu, Nov 14
Wed, Nov 13
Mon, Nov 11
@Bawolff - well, it's been a long time. But I just checked in a change that I think fixes the 4th major security issue listed, the one that starts with "<div data-bar". The change can be found here:
Sun, Nov 10
I finally removed support for MW < 1.28 from Page Forms a few days ago, so now you could say that Page Forms no longer supports PHP 5.3 by proxy. (Actually, it no longer supports any version of PHP below 5.5.9, for the same reason.) But what was this request specifically for - replacing "array()" with ""?
Nov 8 2019
Nov 4 2019
Oct 31 2019
Oct 28 2019
@Tgr - thanks, that's helpful. It's great to know that everything I need can be accomplished with this new hook. I submitted a patch for the DifferenceEngine.php bug here:
@Samwilson - thanks for the fix!
Alright, I'll submit a patch for that DifferenceEngine.php bug.
Oct 27 2019
Oct 24 2019
Oct 23 2019
Oct 17 2019
Setting to "Resolved" - feel free to re-open if this is still an issue.
I'm guessing that this has been fixed with Approved Revs 1.2... feel free to re-open if not.
Sorry about the problem, and thanks for the fix! I'm assuming all of this works now.
Oct 16 2019
Oct 14 2019
Fixed now, I guess? Thanks for the patch, and sorry for the delay!
Oct 10 2019
Oct 8 2019
Oct 7 2019
Oct 3 2019
Oct 2 2019
@KatkovYury - sorry for the long delay. This was actually a Page Forms bug. I believe it's fixed now, though I can't test it because it only happens when SMW is installed. I'm setting the bug to "Resolved" - please re-open it if the fix didn't actually work.
Sep 30 2019
I forgot to say - thank you, @Legoktm, for your help with improving the security of all three extensions.
Okay, I believe all three patches are now merged in. In the case of Approved Revs, the code checked in was simpler than the original patch because support for MW < 1.28 was removed, soon after the original patches were created.
Sep 29 2019
That's a good idea - I added in this code in 0069ae1265dc.
Sep 27 2019
@Aklapper - sorry for not responding here before. I believe I took care of the Cargo patch with the commits 9b8d3a0a2d10 and 3407fb767040. I still need to take care of the Approved Revs and Admin Links patches.
Sep 11 2019
Sep 9 2019
Sep 6 2019
Sorry about that! This was a bug from January. I just checked in a fix for it; hopefully it will get to you soon.
Wow! It looks like that bug has been in place for a long time. Sorry about that. I just checked in a fix - hopefully it will get to you soon.
Sep 5 2019
That seems like the right approach, yes. I think someone else made that request too recently, but I can't remember where. Hopefully this is easy to do.
Sep 4 2019
I should probably know this, but how do you restrict an entire form?