MediaWiki talk:Gadget-ListingEditor.js

From Wikivoyage
Jump to navigation Jump to search

Suggested modification[edit]

Apply this change. The first is for speed increase and the second is for XML syntax compliance. --Andyrom75 (talk) 17:20, 20 November 2014 (UTC)[reply]

I applied the second change for XML syntax compliance [1], but I'm not sure I understand the first - document.getElementById() is a fast call on all modern browsers, so why would a custom JQuery selector be better? -- Ryan • (talk) • 17:46, 20 November 2014 (UTC)[reply]
Because document.getElementById() look the whole document for something, while the "[..]" is a conversion of a string.
Regarding the performance, I suggest also to apply this cahnge, because after the conversion of this script into a gadget, importStylesheet is not used anymore, so there's no delay effect. --Andyrom75 (talk) 17:56, 20 November 2014 (UTC)[reply]
Yes, but $("[id='"+key+"']") is still going to try to initialize a JQuery element, and to do so will scan the entire document looking for a DOM element that matches the "id=key" selector - or am I misunderstanding? -- Ryan • (talk) • 18:06, 20 November 2014 (UTC)[reply]
Maybe you have overlooked a detail. The current script scans the whole source twice instead of just once. I suppose that this choice was made by the fact that the simple $("#"+key+"") doesn't work when key contains "non-standard chars", so he has decided to look for the id with document.getElementById() then extract a portion of code that can be searched with $() without caring of the "non-standard chars". Let me know if I've clarified your doubt. --Andyrom75 (talk) 19:13, 20 November 2014 (UTC)[reply]
I'm not a JQuery expert, so if the following isn't a concern then I'll apply the patch. I'm not sure I understand what you mean when you say "the current script scans the whole source twice instead of just once". $(document.getElementById(key)) should just get the DOM element and then use that element to initialize a JQuery object - one scan using a fast native method. $("[id='"+key+"']") needs to get that same DOM element by doing a text scan of the document for a DOM element that matches "[id='"+key+"']", and then use the resulting DOM element to initialize the JQuery object, which seems to me like it should be slower than the original code. Does that make sense? -- Ryan • (talk) • 19:55, 20 November 2014 (UTC)[reply]
Ryan I think you are right, the $() that enclose document.getElementById(key) just trasform the string into an object. Forget about this change. Consider to implement the one relevant to the importStylesheet (see above). --Andyrom75 (talk) 21:02, 20 November 2014 (UTC)[reply]
You might have provided the wrong diff with respect to the stylesheet - this diff just removes a comment and a font color. Is there another change related to stylesheets elsewhere? -- Ryan • (talk) • 23:10, 20 November 2014 (UTC)[reply]
Originally the css was loaded through importStylesheet(), so the css' were loaded in series instead of in parallel as it happens now with the gadget structure. Because of this, the layout of the article was rendered with a delay (you can emulate it). Now there's no more that rendering issue (graphic performance), so we can use directly the class instead of writing inline the style. By now I've just eliminate one of the two styles according to the existing classes. To eliminate also the other one I think that we can apply this change in the .css file: ".ui-dialog {" -> ".ui-dialog, span.vcard-edit-button a {". --Andyrom75 (talk) 08:01, 21 November 2014 (UTC)[reply]
I've made the suggested change to the Javascript. -- Ryan • (talk) • 17:16, 21 November 2014 (UTC)[reply]
In the same spirit is possible to apply the following changes: on js and on css.
I've also noticed a couple of bugs:
  1. On IE8 do not appear the various [add listing] links
  2. On Chrome (since several weeks), the second field on the right side (Website on it:voy or Alt in en:voy) has a bad alignment. Setting the browser zoom at 75%, the alignment became correct.
For the first bug I have no solution, while for the second I've patched in this way. However I'm not sure if it's the best way to solve it or if it has any downside even with other browsers. What do you think? --Andyrom75 (talk) 13:47, 22 November 2014 (UTC)[reply]
I've tested the patch with Chrome, FF and Opera. In these 3 everything is fine. I wasn't able to test it on IE because of bug #1. However I'm confident that it will work on that browser too. --Andyrom75 (talk) 21:43, 22 November 2014 (UTC)[reply]
There have been a few diffs linked in the discussion above and I've lost track of which ones are tested and ready to apply. Can you provide diffs of what still needs to be changed and isn't still being tested? Thanks! -- Ryan • (talk) • 20:06, 23 November 2014 (UTC)[reply]
You can apply all the three diff posted on "13:47, 22 November 2014". For the IE8 bug I don't have a solution at the moment. --Andyrom75 (talk) 22:07, 24 November 2014 (UTC)[reply]
I've gotten rid of the inline CSS from MediaWiki:Gadget-ListingEditor.js [2], although rather than have the styles for "span.vcard-edit-button a" defined in two places I re-used the existing style definition: [3]. I'm not entirely sure I understand why a line height would be the correct solution to an alignment issue ([4]) so I'm a bit hesitant to apply that change - I would think that adding a "clear: both" to the "#listing-editor .input-text" style might make more sense, although I only tested on Chrome. -- Ryan • (talk) • 01:32, 25 November 2014 (UTC)[reply]

┌────────────────────────────────────────────────────────────────────────────────────────────────────┘ Much more better your modification on css. Honestly speaking I don't know why I did something different :-D Thanks for highlight it to me. Regarding the line height fix, I've found it after several tries and I've noticed that it works with all the browsers. Maybe when I'll have some spare time I'll test your solution too (unless you could do it earlier than me). PS Test it also with different zoom levels. --Andyrom75 (talk) 21:37, 25 November 2014 (UTC)[reply]

I applied the "clear:both" change [5], which makes more sense to me and seems to work at different zoom levels in Chrome. I'll do more testing on other browsers as time allows. -- Ryan • (talk) • 21:57, 25 November 2014 (UTC)[reply]
Tested on FF and it works. On IE8 the JS mask doesn't work (as per the bug mentioned above). Any idea on its resolution? --Andyrom75 (talk) 23:09, 27 November 2014 (UTC)[reply]

Suggested modification 2[edit]

The condition 

$('#mw-diff-ntitle1').length

is not 100% correct because it prevents on the "Latest" diff, to add the modification links (where are supposed to be). The patch I've found is the following: 

($('#mw-diff-ntitle1').text().search('Latest') === 0)

but I don't like that it depends on the language in use on a specific wiki.

Any idea for a broader patch directly applicable to any language? --Andyrom75 (talk) 12:10, 5 December 2014 (UTC)[reply]

Found it. However, better ideas are more than welcome.
($('#mw-diff-ntitle4').text().trim().length > 0)
Here the implemented change on it:voy. PS I've added also a missing semicolon. --Andyrom75 (talk) 18:00, 5 December 2014 (UTC)[reply]
I haven't tested the following yet, but would it maybe make more sense to use the page config variables to compare whether the current page is the current revision instead of relying on UI elements that aren't guaranteed not to change? Something like: 
mw.config.get('wgCurRevisionId') != mw.config.get('wgRevisionId')
The benefit of that approach is that it is a bit easier to understand what it's doing, and it shouldn't break if they change the HTML in a future upgrade. -- Ryan • (talk) • 16:46, 6 December 2014 (UTC)[reply]
I like it; much more better. Just tested: it works perfectly. --Andyrom75 (talk) 18:26, 6 December 2014 (UTC)[reply]
Thanks for testing - I've applied the change [6]. -- Ryan • (talk) • 18:55, 6 December 2014 (UTC)[reply]

Suggested modification 3[edit]

Time ago I've written in Wikivoyage:Listing editor how to fix a bug: "Saving a listing without a fax number, it add a blank space at the end of the line" with this patch. I think it can be added also because it provides a benefit side effect (clean all the useless blanks at the end of any row). --Andyrom75 (talk) 10:10, 6 December 2014 (UTC)[reply]

I only use regular expressions at a very basic level, so unfortunately I'm not qualified to review that patch. Would there be any downside to using $.trim()? I'd be more comfortable applying a patch that used a method I was familiar with, otherwise hopefully someone else can take a look at this patch and apply it. -- Ryan • (talk) • 17:17, 7 December 2014 (UTC)[reply]
I'm not totally sure, but I think that trim is not applicable because trim should work with a single string while in this context, sectionText is the whole document. The "gm" statment in the regex is used to apply the the regex over a (m)ultiline text and that the replacement may occure more than one time, and this is called (g)reedy. --Andyrom75 (talk) 20:01, 7 December 2014 (UTC)[reply]
Ryan, I don't know if you have got the chance to review the patch, however, considering that in it:voy is in use since last June, you could add it without any risk. --Andyrom75 (talk) 08:11, 22 December 2014 (UTC)[reply]
Thanks for the follow-up. Given my lack of regular expression expertise I'm not comfortable applying that patch, but would be happy for anyone who understands it better to do so. -- Ryan • (talk) • 20:46, 22 December 2014 (UTC)[reply]
Ryan, no problem. Any clue on who is a regex expert within the en:voy admin? --Andyrom75 (talk) 16:02, 24 December 2014 (UTC)[reply]
I'd suggest requesting feedback in the Pub. I trust you that the fix works fine on itvoy, my concern is mainly that I don't understand what it's doing well enough to know whether a simpler solution might be possible, and thus feel a little odd being the one to apply it. Hopefully someone who has more experience with regular expressions can comment. -- Ryan • (talk) • 22:31, 24 December 2014 (UTC)[reply]

Alternate fix[edit]

I've implemented an alternate fix for this issue: Special:Diff/2695941/2719886. The problem seems to be that the flag for the "fax" field to signal an end-of-line wasn't set, but by chance a newline was usually added immediately after "fax" whenever the "image" field was empty. This change sets "fax" to signal end-of-line, and puts "image" on its own line. Tested briefly here, but the change seems straightforward to me. -- Ryan • (talk) • 18:40, 24 January 2015 (UTC)[reply]

On it:voy it doesn't work due to some differences in the code (I haven't investigated too much), so I've kept the previous fix. --Andyrom75 (talk) 11:05, 27 February 2015 (UTC)[reply]

dot notation vs bracket notation[edit]

The wikieditor suggest to convert all the bracket notation (with constant name ... i.e. labeled) into a dot notation. On internet I've found some pages that support this approach. I don't think it's a matter of performance because I've seen that they changes significantly from browser to browser. So I suppose that it's just to compress the code and readability (although I'm more used to the old style :-P).

Any comment about it? Alias: change or keep it? --Andyrom75 (talk) 19:06, 6 December 2014 (UTC)[reply]

Per [7] it sounds like using one or the other is mainly a matter of coding style preference - the advantages cited for bracket notation strike me as fairly low importance. Does the wikieditor cite any reason for switching? -- Ryan • (talk) • 17:20, 7 December 2014 (UTC)[reply]
No explanation provided. An example message is the following: "Info: ['cities'] is better written in dot notation.". --Andyrom75 (talk) 19:46, 7 December 2014 (UTC)[reply]

Problems with images or links in the content field[edit]

Ryan (or anyone else), I've found a couple of problems but that are not so easy to be patched, so I'm writing here trying to find support on their resolution.

Click on the edit link of the following listings and you'll see that the link will disappear and the code of the image will be broken

  • 1 Name1, address1. Beginning text it:voy link ending text. 
  • 2 Name2, address2. Beginning text link it:voy ending text. 

Any idea? PS To activate the [edit] link those listings should be move out from a talk page, so take a look at this page. --Andyrom75 (talk) 11:40, 27 February 2015 (UTC)[reply]

I was planning on working on some updates to the listing editor this weekend, so I'll take a look while I'm working on those changes. Thanks as always for collaborating on getting this fixed. -- Ryan • (talk) • 15:55, 27 February 2015 (UTC)[reply]
As you noted on my talk page, this functionality seems to work fine on English Wikivoyage. I did a diff of the Italian & English listing editor code, and after a quick review of the code changes the only differences that looked to me like they might be related to this issue are the following:
  1. Italian Wikivoyage uses a regex to convert the listing content prior to saving (saveForm(summary, sectionText.replace(/ +$/gm,""), number, , );).
  2. Italian Wikivoyage uses "===" or "!==" in a number of places where English Wikivoyage uses "==" or "!=". These comparisons will be the same in most cases, but the Italian version will fail if a type conversion is needed.
I'm not sure if either of the above are the cause of the problem - as noted previously, I'm not skilled enough with regular expressions to be fully confident of what that express is doing, and I didn't see any obvious reason why type conversion would be needed in any of the comparisons that were changed - but those were the differences that I noticed that looked like they might be triggered when editing an existing listing. -- Ryan • (talk) • 18:13, 1 March 2015 (UTC)[reply]
The problem shouldn't be on point 1 because the issue arise during data collection so at loading and not saving. I've tried also to replace "===" with "==" and "!==" with "!=", but the problem still remains. Further suggestion? --Andyrom75 (talk) 21:35, 2 March 2015 (UTC)[reply]
In the listings on User:Andyrom75/Sandbox‎‎ the problem is due to having equals signs ("=") in the template content. Something in the "content" field is also being parsed in a way that is throwing off the code that adds event listeners to the edit links. I will need to think about it for a bit to see what a good solution might be. -- Ryan • (talk) • 05:54, 3 March 2015 (UTC)[reply]
Thanks for taking care. Keep me posted. --Andyrom75 (talk) 08:45, 3 March 2015 (UTC)[reply]

┌────────────────────────────────────────────────────────────────────────────────────────────────────┘

@Andyrom75: Special:Diff/2750058/2750076 should resolve the issue with listings that contain wikitext in the name being impossible to edit (Not ''Working Test'' from User:Andyrom75/Sandbox‎‎). I used the fix from @Mjbmr: that is already live on fa:مدیاویکی:Gadget-ListingEditor.js. The issue with listings that contain extra equal signs is still TODO. -- Ryan • (talk) • 03:13, 19 March 2015 (UTC)[reply]

Thanks Ryan. I've just added this patch on it:voy too. I'll keep on waiting for the "extra equal" one. --Andyrom75 (talk) 08:43, 19 March 2015 (UTC)[reply]

Price range[edit]

Considering that hotels & restaurants are tipically (to not say always) split in the three price ranges, could be useful to add for this two listing types the possibility to select that range in the mask so that this new location would be added directly under this subsection (creating it when missing) instead of creating first one then the other? --Andyrom75 (talk) 09:23, 28 February 2015 (UTC)[reply]

Summary of recent changes[edit]

There have been a few questions about recent changes to the listing editor, so here is a summary:

I think that's everything. Ping me with any questions. -- Ryan • (talk) • 02:13, 1 March 2015 (UTC)[reply]

One more:
-- Ryan • (talk) • 07:27, 2 March 2015 (UTC)[reply]

Is this necessary?[edit]

string = string.replace(/{{vCard/g,'{{listing'); --Andyrom75 (talk) 21:39, 2 March 2015 (UTC)[reply]

Ryan, do you think that nowadays is the above code still really necessary? --Andyrom75 (talk) 08:44, 19 March 2015 (UTC)[reply]

Multiline content listings[edit]

Discussion moved from User talk:Wrh2#Multiline content listings

Hi Ryan, I've got a couple of questions upon this test I've made.

I've notice that here in en:voy (differently from the test I've made on it:voy, the "[edit] link" is correctly positioned at the end of the content, while on it:voy, it's wrongly positioned at the first "newline". Which is the patch to apply? I remember that I've discussed about it with Torty and he hasn't a solution.

On the other hand, here in en:voy, although aesthetically everything is well positioned, the mask doesn't show up on click. Maybe the above mentioned patch has some side issue? --Andyrom75 (talk) 15:32, 21 March 2015 (UTC)[reply]

The matter, should be located in the function addEditButtons inside MediaWiki:Gadget-ListingEditor.js. --Andyrom75 (talk) 15:45, 21 March 2015 (UTC)[reply]
Which is the purpose of this cycle:
            $(EDIT_LINK_CONTAINER).each(function() {
                if (!isElementEmpty(this)) {
                    $(this).append(' | ');
                }
            });
? --Andyrom75 (talk) 16:02, 21 March 2015 (UTC)[reply]
Multiline content has also a side issue related to the indentation of the content itself. Look the difference between third and fourth test on my en:voy sandbox. --Andyrom75 (talk) 16:39, 21 March 2015 (UTC)[reply]
I modified Template:Listing when we added the "last edited" field to include a "listing-metadata" span that wraps the "edit" link and the "lasted edited" text:
<!--
Metadata - last edit date, "edit" link, etc
--><span class="listing-metadata">{{#if:{{{lastedit|}}}
| (
}}<span class="listing-metadata-items">{{#if:{{{lastedit|}}}
|<span class="listing-lastedit">updated {{#time: M Y|{{{lastedit}}}}}</span>
| 
}}</span>{{#if:{{{lastedit|}}}
|)
}}</span>
When the addEditButtons function runs it puts the "edit" link inside of the "listing-metadata-items" class, prepending a " | " if that span already has content.
I'm not sure what you mean when you say "mask doesn't show up on click". Can you explain?
The issue with multiline content is due to the list being closed when it encounters a newline. It's worse with indented listings:

::* {{listing | name=This listing is indented and has multiple lines | content=Blah blah blah blah blah blah blah blah blah. Blah blah blah blah blah blah blah blah blah}}

... which generates:
  • This listing is indented and has multiple lines. Blah blah blah blah blah blah blah blah blah.

Blah blah blah blah blah blah blah blah blah 

-- Ryan • (talk) • 17:03, 21 March 2015 (UTC)[reply]
Ok, I think I've understood the idea of your change.
Regarding the bug, as written in the top of this message, go to my sandbox and click on the edit of the second test. It doesn't work because of a JS error because sectionText.match(listingRegex) is executed when listingRegex is null.
Regarding the "newline vs indentation" do you think that there's a remedy? The current layout it's really ugly to see. --Andyrom75 (talk) 18:16, 21 March 2015 (UTC)[reply]
I haven't had a chance to look at the issue where two listings with the same name create issues, but I have it on my TODO list. This is going to be a busy week so I'm not sure when I'll get to it. The multi-line listing issue isn't one I'm likely to look at right now since, as far as I know, it's been a problem since listings were first introduced; perhaps someone else can investigate that one and see if there is a decent fix. -- Ryan • (talk) • 16:43, 23 March 2015 (UTC)[reply]
Ryan, for sure I've express myself in a not very clear way. The above mentioned bug is not related to two listings with the same name, but it has been introduced with the patch that you have explained above, and it occur on listing with multiline content. If you try to click [edit] on those listing, a JS error is generated and the mask not shown. Please let me know if you have understood what I meant. PS I've just changed the name to make it clearer; click on [edit] of Name2 and Name3. --Andyrom75 (talk) 18:31, 23 March 2015 (UTC)[reply]
Based on mw:Help:Lists#Paragraphs in lists I think the correct solution is to edit the listing content, rather than trying to figure out a Javascript "fix". In your test example Mediawiki is closing the list completely, so the badly indented content is no longer a part of the listing (it is outside of the "vcard" span). -- Ryan • (talk) • 05:31, 24 March 2015 (UTC)[reply]
I would agree, in fact in this case I suggest to revert or adjust the above patch to restore the original position of the [edit] link, infact with the previous code, everything works correctly. I suppose that you can leave the recent "updated" information there, because it shouldn't have any effect on the mask. --Andyrom75 (talk) 07:45, 24 March 2015 (UTC)[reply]
Special:Diff/2750076/2753156 will prevent an "edit" link from being added to multi-line listings. I think that may be the best solution, since the listing editor won't be able to properly load them. Having the "edit" link present for these listings, even if it only loads the content that is within the "li" tag, would leave stray content, so forcing users to edit those listings by hand seems like it might be the right thing to do. -- Ryan • (talk) • 03:04, 25 March 2015 (UTC)[reply]
In it:voy we still have the old version (without lastedit feature) where the mask works properly also with multilline content. With works properly I mean that load and save the whole content. I think that is important (especially for less skilled users) that each listing will have its own "edit" link. Maybe you can restore the old behaviour for the "edit" link. What do you think? --Andyrom75 (talk) 08:20, 25 March 2015 (UTC)[reply]
The old code broke with listings that contained any HTML in the title (bold, italic, etc). If there is a desire to return to the old code then go ahead and revert, but my personal opinion is that the new approach implemented by Farsi Wikivoyage is more robust and that it will be easier to address future issues with this code. -- Ryan • (talk) • 03:40, 26 March 2015 (UTC)[reply]
Please look at it:voy. I've implemented the patch for the HTML in the title (and it correctly works) but not the lastedit. The problem is on moving [edit] link from its previous position (just after the first newline) to the new one (at the end of the listing). So the patch to be (at least partially) reverted here on en:voy is this one. --Andyrom75 (talk) 07:54, 26 March 2015 (UTC)[reply]

Please let me know if the requirement is clear or if you need further information. I'd like to keep aligned (as far as possible) the script logic between language version, but I would avoid to propagate a bug. Thanks, and sorry for stressing with this topic. --Andyrom75 (talk) 22:47, 1 April 2015 (UTC)[reply]

Kind reminder :-) --Andyrom75 (talk) 08:02, 25 April 2015 (UTC)[reply]
While I very much appreciate the desire to make multi-line listings editable again, I feel like the current code is OK as-is. I appreciate that Italian Wikivoyage implemented a different approach, but I think the trade-off of having simpler code and a more obvious HTML structure outweighs the disadvantages of being unable to edit multiline listings using the listing editor, particularly since that issue is due to a shortcoming in the Mediawiki list syntax and represents a very, very small percentage of the total listings on the site. The next time we are making significant changes to the listing editor I'll be happy to revisit, but for now I feel like the current version is acceptable. Others may feel differently, so I would not object at all if you wanted to solicit fixes or opinions from others, but for now I personally don't think further changes are a priority. -- Ryan • (talk) • 17:00, 25 April 2015 (UTC)[reply]
Thanks a lot for your clear and polite answer. I just hope in the future you'll find some spare time to fix it. Sorry for bothering you :-) --Andyrom75 (talk) 07:13, 8 May 2015 (UTC)[reply]

Suggested fix[edit]

I think Special:Diff/2740454/2797590 will allow a partial fix to this issue by wrapping the listing in a <div class="listing-item"> tag. That should allow us to change the EDIT_LINK_CONTAINER = 'span.vcard span.listing-metadata-items'; value to EDIT_LINK_CONTAINER = '.listing-item span.listing-metadata-items';, which will catch properly-formatted listing items that contain multiple lines. Unfortunately it won't catch improperly formatted listings (those that are formatted in a way that causes the list to close prematurely), but it's a move in the right direction and a relatively simple fix. I haven't implemented it yet as we need the cache to update existing pages that use Template:Listing first, but I think it should fix some cases where currently the listing editor isn't adding an "edit" link. -- Ryan • (talk) • 02:20, 28 May 2015 (UTC)[reply]

The change is now live. I didn't see any issues in my quick & dirty tests on the Culver City and Theodore Roosevelt National Park articles, but if anyone runs into any problems please revert Special:Diff/2753156/2799815. Note that if anyone wants to copy this change to another language version, both of the following changes are required:
  1. Special:Diff/2740454/2797590 - update Template:Listing to wrap the listing in a div.
  2. Special:Diff/2753156/2799815 - update the listing editor Javascript to look for the new div wrapper instead of the old span wrapper (which only wrapped a single paragraph of the listing).
-- Ryan • (talk) • 15:11, 31 May 2015 (UTC)[reply]

Multiline issue persist[edit]

  • Name1, here1. This singleline listing CAN be edited. 
  • Name2, here2. While this multiline listing

CANNOT

be edited. 

See this test sandbox page. --Andyrom75 (talk) 07:24, 17 June 2015 (UTC)[reply]

Understood. As noted, the fix above only resolves properly-formatted multi-line listings (those that use <p> tags and don't cause the listing tag to close), but will not work for improperly formatted listings. It's a "move in the right direction", but not a perfect fix. -- Ryan • (talk) • 13:40, 17 June 2015 (UTC)[reply]

Certainly, I am seeing issues created by wrapping entire listings in <div> tags. The tags force a line break. Hopefully any actual placenames in-line with the text use {{marker}} instead of {{listing}}, so 1 Ingleside avoids the {{listing}} tag entirely, but the issue crops up with embassy-style venues which often have * [flag] {{listing}} and looked like:

  • Dominion of Newfoundland High Commission, 1916 Beaumont Hamel Dr. God guard thee, Newfoundland.

The added line break turns these into:

  • Dominion of Newfoundland High Commission, 1916 Beaumont Hamel Dr. God guard thee, Newfoundland.

which looks a bit odd. It doesn't affect every city, as only the capitals and a few large cities attract an embassy row, but where it does appear there's usually a string of them in a row. K7L (talk) 17:42, 17 June 2015 (UTC)[reply]

That's not good, and may mean that the fix above needs to be reverted (San Francisco#Cope shows the behavior you've cited, for reference in a live article). I'll revisit after work today. -- Ryan • (talk) • 17:50, 17 June 2015 (UTC)[reply]
Changes reverted. San Francisco#Cope is back to normal, but multiline listings like the Maltese Cross Cabin in Theodore Roosevelt National Park#See are again uneditable with the listing editor. -- Ryan • (talk) • 02:28, 18 June 2015 (UTC)[reply]
Are the flags really necessary? I'd rather have multiline listings working than what appear to be primarily decorative flag images. Powers (talk) 14:39, 18 June 2015 (UTC)[reply]
Perhaps the flags should be a field within the {{listing}}, something like {{listing|name=Dominion of Newfoundland|flag=Dominion of Newfoundland Red Ensign.svg}}? That would require fixing the listing editor so that it stops covertly removing any listing field it doesn't recognise, and would require updating hundreds (though not thousands, as this hits mostly capital or large cities) of articles which already have an "embassy row" with the flags. It is a special case, as it affects just embassies, consulates, high commissions and their ilk. K7L (talk) 16:22, 18 June 2015 (UTC)[reply]

Extra fields[edit]

continued from above

If there are listing template fields that the listing editor shouldn't remove ("phonextra" and "wikipedia" are two that are often included but not displayed) then it's relatively easy to modify the code to make that happen. I think there is value in automatically removing bogus fields ("tags", "facebook" and "skype" are some that I see occasionally, as well as misspellings such as "toll-free" instead of "tollfree") since that is essentially garbage data, but if there are fields that the listing editor shouldn't remove please call them out and we can modify the listing editor code to leave them in place. -- Ryan • (talk) • 18:16, 18 June 2015 (UTC)[reply]

I think any field that was potentially in our imported data should be retained. The canonical list is still here and includes 'phoneextra' (note the second 'e'), 'hoursextra', 'priceextra', and 'tags'. 'Geo' is also there but it was deprecated when 'lat' and 'long' were added. Powers (talk) 23:35, 18 June 2015 (UTC)[reply]
I think supporting the legacy attributes and the occasionally-included "wikipedia" attribute makes sense for the most part, although I'm hesitant about the legacy "tags" attribute. I always remove it when I see it in use and have yet to see a case where it was useful - typically I'll see a listing for a Marriott hotel, and the tags are inevitably "hotel,marriott,city-that-hotel-is-in" or something similar to that. If people want to see that info maintained I don't feel strongly on this point, but my preference would be to get rid of it since it's essentially confusing clutter that doesn't add any value that I can see. -- Ryan • (talk) • 02:38, 19 June 2015 (UTC)[reply]
The listings editor is not the appropriate tool to do this for one simple reason: it's running not under an account which legitimately belongs to its creator, but under the userid of some other random editor who is trying to make other changes to the listing. That user did not request the fields be removed, is not informed that fields are being removed but at the same time is held responsible because the edits were made in his/her name. That makes this a w:Trojan horse (computing). I refuse to use this script. Anyone who wants to engage in automated removal of information should go to Wikivoyage:Script nominations, get consensus first and then make the edits under their own name from an account which they legitimately control. Anything else is sneaky and underhanded.
That said, the proper response to "tollfree" vs. "toll-free" vs. "freephone" vs. whatever is to correct the field name, not to silently discard valid telephone numbers. That will likely require manual intervention for anything more than the most basic lists of common misspellings. Fields which exist in other-language Wikivoyages (Facebook, mobile telephone, a few others) should be left alone as - even if the info is of no immediate use to us - it's needed when translating the listings into those other languages. Fields which were proposed but not yet implemented (such as "lat, long" when we originally forked in 2012 - they're valid now but were ignored then, or "wikidata" - where there's been plenty of discussion but nothing actually done yet) should be left alone. Fields with simply-wrong names need the information moved to the correct name, not deleted by a sneaky script with the changes blamed on some random bystander. The only way to know whether "tags=" contains salvageable info or rubbish is to actually manually look at the text - it's not something that a script blindly removing fields can be entrusted to do, as that throws out the good with the bad. The current implementation is a bug, not a feature. K7L (talk) 15:52, 20 June 2015 (UTC)[reply]
@K7L: I've been working on a 2.0 version of the listing editor that will no longer delete unrecognized listing template parameters if they have a value - unrecognized listing template parameters will still be removed if they do not have a value, which seems like a safe cleanup to me, although if there is an argument to be made for keeping unrecognized empty fields then it would be easy to change that behavior. If you (or anyone else) are interested in trying out the new listing editor you can do so by going to "Preferences" → "Gadgets" and then un-selecting the existing "ListingEditor" in the "General" section (required - both old & new listing editors cannot be active at the same time) and then enabling "ListingEditor2 (beta)" from the "Experimental" section. Other changes in the new version are called out at Wikivoyage:Listing editor#Changelog. I'm still testing this new version out and will be making additional changes to the code (mainly to make it easier for the new script to be used in other languages), but the new version has worked well for me over the past several days. -- Ryan • (talk) • 18:17, 8 July 2015 (UTC)[reply]
I would create a list of "tolerated parameters". The language versions that do not want this behaviour (i.e. use sole and only the standard parameters) can empty this array, without touching the code. On the other hands, the others can just modify the list, without the risk of keeping not existing or mistyped parameters, or maybe (like sometimes happens) copied and pasted from a language version to another. --Andyrom75 (talk) 21:10, 8 July 2015 (UTC)[reply]
Rather than creating another list of tolerated parameters, which would still require explicit approval of all acceptable listing parameters, this change adds a ALLOW_UNRECOGNIZED_PARAMETERS flag which allows either all non-empty parameters to be kept, or (if false) allows only those parameters that are supported by the listing editor to be kept. I think K7L makes a good point that the listing editor shouldn't be removing values without a user knowing that it is happening, particularly in cases where a typo ("toll-free" vs "tollfree") might result in valid data being lost. However, you also make an excellent point that any language version that wants the old behavior should be able to enable it, and I this change should supports that. -- Ryan • (talk) • 21:29, 8 July 2015 (UTC)[reply]
For me it's fair enough because I'm in favour of the standard behaviour; the other current approach risk to keep mispelled parameters forever. IMHO its deletion from the tool would be easier noticed in the recent changes page. However, if you are confident that this is the best approach when ALLOW_UNRECOGNIZED_PARAMETERS = true, just keep it as it is. --Andyrom75 (talk) 07:02, 9 July 2015 (UTC)[reply]

Minor code simplificaiton[edit]

Considering that the template vCard does not exist, I think that this line can be deleted. 

string = string.replace(/{{vCard/g,'{{listing');

I also suppose (although not sure 100%) that the first instruction can be deleted, because in the sample tests I've done it returns always undefined and is the second one that gives a value to the variable link. 

var link = entry.find( '.mw-editsection a' ).attr( 'href' );
if (link === undefined) link = entry.closest('div.mw-h2section').find( '.mw-editsection a' ).attr( 'href' );

so I would replace the previous two instructions with the following one: 

var link = entry.closest('div.mw-h2section').find( '.mw-editsection a' ).attr( 'href' );

Can anyone give a look at them and in case update the script? --Andyrom75 (talk) 13:50, 20 June 2015 (UTC)[reply]

The vcard replacement has been removed. I still need to take a closer look at the heading change - I'm not sure if that pattern is used with a different skin or why it's there - although if anyone else can verify that it's safe to remove the first change please go ahead and do so. -- Ryan • (talk) • 01:39, 25 June 2015 (UTC)[reply]
Ryan, I've made other tests that seem to be independent from the skin, here below the results:
Clicking on [edit]
of an existing listing		 Clicking on [add element]
of a section title
First istruction undefined target
Second istruction target target
Basically the first instruction is redundant, because the second one reach the target in every context. --Andyrom75 (talk) 12:54, 25 June 2015 (UTC)[reply]
Yes Done - Special:Diff/2818518/2818519. -- Ryan • (talk) • 06:04, 30 June 2015 (UTC)[reply]
@Andyrom75: - while testing some changes I found that with the above modification the "add listing" link for sub-sections (h3) was adding the listing to the main section (h2) rather than the subsection, so for now I've restored the original code. -- Ryan • (talk) • 17:26, 5 July 2015 (UTC)[reply]
Ryan, thanks a lot for noticing it. I think I've tested it only with listing in the main section. I'll revert it on it:voy as well. --Andyrom75 (talk) 17:41, 5 July 2015 (UTC)[reply]

Commented listing bug[edit]

When a listing is commented by a user (for whichever reason) the following listings became "unmanageable" because the numbering system became disaligned between "what you see and what you get". See this example and try to edit the second listing (i.e. listing number 4).

A patch for this can be the following (already implemented time ago on it:voy): Change: 

function getSectionText(number) {
        var wikiText = $.ajax({
            url: mw.util.wikiScript(''),
            data: { title:mw.config.get('wgPageName'), action:'raw', section:number },
            async: false,
            cache: false // required
        }).responseText;
        return wikiText;
    }

into: 

function getSectionText(number) {
var wikiText = $.ajax({
	url: mw.util.wikiScript(''),
	data: { title:mw.config.get('wgPageName'), action:'raw', section:number },
	async: false,
	cache: false // required
}).responseText;
var comments = wikiText.match(/<!--[\s\S]*?-->/mig);
if ( comments !== null ) {
	for(var i = 0; i < comments.length; i++ )
	{
		var comment = comments[i];
		var rep = '<<<COMMENT' + i + '>>>';
		wikiText = wikiText.replace(comment, rep);
		replacements[rep] = comment;
	}
}
return wikiText;
}

and add the following code: 

for (var key in replacements) {
	var val = replacements[key];
	sectionText = sectionText.replace(key, val);
}
replacements = {};

inside the fuction formToText between the following two lines: 

var summary = '/* ' +upperCaseFirst($("#input-type").val()) + ' */ ';
if (mode == 'add') {


This is a very "quick & dirty" patch that perfectly works. Any style or solidity improvement is more than welcome. --Andyrom75 (talk) 14:10, 20 June 2015 (UTC)[reply]

Yes Done - Special:Diff/2814995/2818518. -- Ryan • (talk) • 06:05, 30 June 2015 (UTC)[reply]

nowiki the code[edit]

To avoid confusion on the maintainance categories a patch similar to this one must be applied. --Andyrom75 (talk) 18:16, 20 June 2015 (UTC)[reply]

My only concern with adding <nowiki> is that I can't find any recommendation to do so in the Mediawiki guidelines, nor did I find any Javascript in the MediaWiki namespace on Wikipedia that does so, so I'm slightly worried about doing something outside of the normal standards if we don't really need to. Again, perhaps others will know better and can make this change with the certainty that it's the right solution to the problem. -- Ryan • (talk) • 01:45, 25 June 2015 (UTC)[reply]
Ryan, is a quite common spread solution used in many wiki scripts. FYI Someone apply also the <pre> tag, but for this purpose is not necessary. As per my experience you can proceed without any worries. --Andyrom75 (talk) 10:53, 25 June 2015 (UTC)[reply]

Fix listings with listing-like template tags[edit]

This change should resolve an issue that caused the listing editor to open the wrong listing when an "edit" link was clicked on Toronto#See. The problem was the {{seeDistricts}} tag, which the listing editor matched as a listing tag, throwing off the matching code and causing the wrong listing to open in the editor. The change should resolve that issue, but if it has any side effects that cause anything else to break please revert. -- Ryan • (talk) • 04:39, 7 July 2015 (UTC)[reply]

Ryan, I haven't checked in detail your new code, but if I've understood correctly the nature of the issue, the problem was in the following line:
var listingRegex = new RegExp("{{\\s*(" + regex.join('|') + ')','g');
because it checks if the template name starts with that name, instead of is equal to that name, so without changing the rest of the code, should be enough to add "\\s*\\|" at the end of the regex:
var listingRegex = new RegExp("{{\\s*(" + regex.join('|') + ')\\s*\\|','g');
I've made just an easy test and it seems to work. If you want we can deeply check it. --Andyrom75 (talk) 07:49, 7 July 2015 (UTC)[reply]
Have you evaluate the differences of the two patches? --Andyrom75 (talk) 07:05, 9 July 2015 (UTC)[reply]
Both approaches should work - your regular expression adds the pipe character as a way of ensuring that partial text isn't matched, while mine is looking for a word boundary. The version I pushed was a backport from the v2 listing editor I'm working on and also included a simplification to the code that searches for the index of the matched text, but that shouldn't be required with your version. -- Ryan • (talk) • 14:38, 9 July 2015 (UTC)[reply]

Find on map[edit]

The link to "GeoMap" should have the following form, if possible:

http://maps.wikivoyage-ev.org/w/geomap.php?lang=language version identifier&page=page name&location=address or name.

Examples:

-- Joachim Mey2008 (talk) 12:04, 9 August 2015 (UTC)[reply]

The above comment was later deleted by an anonymous user - I'm not sure if that user was Mey2008, someone who knows the maps well, or someone trying to cause mischief, so I've restored it pending clarification. The current listing editor geomap format is:
http://maps.wikivoyage-ev.org/w/geomap.php?lat=34.004&lon=-118.401&zoom=15&location=4117%20Overland%20Ave
The anonymous user's revert noted that "deleted: URL to GeoMap now with geo coordinates". Can someone who knows clarify:
  1. Should "lang" and "page" should be added in all cases?
  2. Should "lang" and "page" be added only when we don't have "lat" and "lon" parameters?
  3. Are there any other parameters that should be added or removed?
It's a simple matter to make any required changes, but I'd like to be sure I'm fixing things properly. -- Ryan • (talk) • 16:12, 9 August 2015 (UTC)[reply]
The whole mess I've produced. Sorry. - The parameter "lang" is useful for the later localization. The "page" parameter is useful if coordinates are not available. The parameters "lang" and "page" should be added in all cases. The other parameters can remain unchanged. -- Joachim Mey2008 (talk) 04:24, 10 August 2015 (UTC)[reply]
Thanks for the clarification - I've added the "page" and "lang" parameters (Special:Diff/2836156/2836240). -- Ryan • (talk) • 05:06, 10 August 2015 (UTC)[reply]
Thanks for the quick addition of the parameters. I will adapt the script "geomap.php" in the coming days. -- Joachim Mey2008 (talk) 05:50, 10 August 2015 (UTC)[reply]
I have adapted the script. It now works in most cases. The decisive factor is the data status in OSM. Often there are missing house numbers or the Latinized spelling of street names. - Please report bugs and suggestions. - Joachim Mey2008 (talk) 13:00, 13 August 2015 (UTC)[reply]

EscapeId deprecated[edit]

Ryan (or any other active admin), please apply this change because the current method prevent the reload of the page if we change a listing from the last diff instead of the "current article". --Andyrom75 (talk) 13:58, 5 July 2018 (UTC)[reply]

Done. -- WOSlinker (talk) 17:59, 19 July 2018 (UTC)[reply]

HTML typo[edit]

ARR8, WOSlinker, line 390 should be changed from:

'&nbsp;<a id="coords-null-link" href="javascript:">&#8709;</a>' +

to

'&nbsp;<a id="coords-null-link" href="javascript:">&#8709;</a></td>' +

--Andyrom75 (talk) 07:10, 2 October 2019 (UTC)[reply]

I've updated it. Thanks. -- WOSlinker (talk) 11:49, 2 October 2019 (UTC)[reply]

Old unused code[edit]

WOSlinker, function createRadioOld (lines: 1082-1146) can be removed because it has been replaced by createRadio. --Andyrom75 (talk) 14:23, 18 October 2019 (UTC)[reply]

I've removed from the test/beta version at MediaWiki:Gadget-ListingEditor2.js for now and will do the other if a few days. -- WOSlinker (talk) 10:54, 19 October 2019 (UTC)[reply]
WOSlinker, since it's unused code I don't think there is so much to be tested, but feel free to follow the procedure that you consider more appropriate.
With the same approach, please delete also
			//'lat':			{ 'p':  'P625', 'label': 'latitude', 'fields': 'lat', },
			//'long':			{ 'p':  'P625', 'label': 'longitude', 'fields': 'long', },

inside WIKIDATA_CLAIMS, and substitute 

					if ( (key === 'lat') || (key === 'long') ) {
					/*	if( res[key].value ) {
							msg += createRadio(ListingEditor.Config.WIKIDATA_CLAIMS[key], res[key].value[key+'itude'], res[key].guidObj);
						} */
					}
					else if (key === 'iata') {

with just 

					if (key === 'iata') {

Both codes have been replaced many years ago by the use of "coords".

Thanks, --Andyrom75 (talk) 10:43, 22 October 2019 (UTC)[reply]

MediaWiki:Gadget-ListingEditor.js has now been updated. -- WOSlinker (talk) 18:24, 22 October 2019 (UTC)[reply]

Bug in line 1864[edit]

Sometimes the script fails in line 1864: 

} else if (listingTemplateAsMap[lastKey].length) {

It fails if lastKey is undefined. This happens if the first parameter has no name like 

{{listing | nonsense | type = see | etc. }}

You had to check if lastKey is defined like 

} else if (lastKey && listingTemplateAsMap[lastKey].length) {

--RolandUnger (talk) 17:10, 5 July 2020 (UTC)[reply]

Solved on v2.4 --Andyrom75 (talk) 07:09, 28 July 2020 (UTC)[reply]

Something went wrong[edit]

Swept in from the pub

Looks like sections of country region or city articles can't be expanded for now in mobile WV. Temporary issue due to rollout of new wiki version? TagaSanPedroAko (talk) 03:19, 29 June 2023 (UTC)[reply]

Seeing this as well, mobile Wikivoyage is completely broken for me right now. Desktop renders fine and so do sections of articles without listings, like this one. Jpatokal (talk) 08:42, 29 June 2023 (UTC)[reply]
The same problem at Dutch WV. No problem at WP's. FredTC (talk) 05:37, 30 June 2023 (UTC)[reply]
@PPelberg (WMF) or Jdlrobson will probably want to know about this. WhatamIdoing (talk) 08:34, 30 June 2023 (UTC)[reply]
It seems that there are no problems at the German, Esperanto and Spanish Wikivoyages (same mediawiki and skin versions) but I observed the problem at French and Italian Wikivoyages, too. Expanding and hiding of the article sections is made by a JavaScript. Maybe there are collisions with other scripts (listings, listing editor, JQuery UI). --RolandUnger (talk) 09:30, 30 June 2023 (UTC)[reply]
I'm looking now. This is likely gadgets related as it works with safemode=1 on the URL... will report back soon. Jdlrobson (talk) 13:29, 30 June 2023 (UTC)[reply]
The issue was MediaWiki:Gadget-ListingEditor.js which is now loaded on mobile per tech news. I've filtered it to not load on the Minerva skin and am now in the process of fixing up other projects.
@Andyrom75 are you still the maintainer of this gadget (and the other default gadgets)? I'm a bit concerned about the amount of JavaScript Wikivoyage is loading on page load as it's likely impacting SEO/performance and loading jquery.ui in a default gadget is a big no-no so we should start making plans to move that away. I need to think about strategies to fix this but it would be great if you are if you could subscribe to this ticket in the mean time as I may have some questions!: https://phabricator.wikimedia.org/T340705. Thanks in advance! Jdlrobson (talk) 14:50, 30 June 2023 (UTC)[reply]
@Jdlrobson thanks for the ping and the patch. I'm the last active script maintainer for the listing editor. Unfortunately I don't have enough time for new development, but I can support if there are minor fix to perform here or in it:voy. --Andyrom75 (talk) 20:20, 30 June 2023 (UTC)[reply]
@Jdlrobson MediaWiki talk:Gadgets-definition#Edit_request – Yahya (talk) 11:13, 1 July 2023 (UTC)[reply]
@Jdlrobson In the past, JQuery UI was not loaded on mobile devices (Minerva skin) that means MediaWiki:Gadget-ListingEditor.js was not active. I have no idea why it is now loaded contrary to the recommendation to replace it by OOJS. Otherwise programmers need a really good explanation how to work with the difficult-to-understand OOJS and how to load js tools only on demand. --RolandUnger (talk) 17:01, 2 July 2023 (UTC)[reply]
That entire script needs a thorough rewrite and not only to get rid of jQuery UI. It's loading all the code for the editor on every single page load which is (and has been) incredibly inefficient. It should only load a loader gadgets which detects the various elements that are relevant for the editor and adds a clickhandler and then upon activating the editor should lazy load the editor. I identified this as a problem script a while ago on discord when I noticed what this script was doing and being suprised that such an exceptionally large script was loaded by default for everyone on en.wv. No blame game or anything, but is not a good approach. This was really a user script, that became a gadget that then people turned on by default, without evaluating wether or not it was written in a way to be suitable to be a default gadget. TheDJ (talk) 13:44, 3 July 2023 (UTC)[reply]
@TheDJ: This is correct. Rewriting is necessary. But it would be a huge task to do this (and maybe nobody in the English community can do this). But the script was written at a time at which loader were not available or unknown. There is a similar task at the German Wikivoyage, and I will do rewriting step by step (swapping i18n, loading core code on demand, using OOUI). All together, it will surely take a year to do this. Furtunately, I substantially finished programming the listing modules so I can start rewriting the listing editor script. --RolandUnger (talk) 06:32, 4 July 2023 (UTC)[reply]
I wonder if this task would be a suitable size for a mw:hackathon project. WhatamIdoing (talk) 09:10, 4 July 2023 (UTC)[reply]
Also, @RolandUnger, I think that OOUI is being replaced by mw:vue.js. You might as well leapfrog over OOUI to the new-new thing. WhatamIdoing (talk) 09:11, 4 July 2023 (UTC)[reply]
I will check this hint. --RolandUnger (talk) 04:49, 5 July 2023 (UTC)[reply]
Note gadgets now default to desktop AND mobile so this is what jquery.ui was not loading on mobile before. I am interested in fixing this gadget to at least load when needed per TheDJ suggestion. It would also be a good idea to have one place to maintain this library since there seem to be several forks. I understand the markup varies per wiki but perhaps that could be achieved by different means - e.g. MediaWiki message. Jdlrobson (talk) 15:52, 4 July 2023 (UTC)[reply]
Loading the script on demand is one of my first goals, too. At the German Wikivoyage I forked the tool about 6 years ago, and now there is a marked difference between the wikis. Both the listing template and editor were permanently enhanced. --RolandUnger (talk) 04:49, 5 July 2023 (UTC)[reply]
A new fork is in progress. You can test it out by disabling the existing ListingEditor gadget and including the following code:
importScriptURI('https://en.wikivoyage.org/w/index.php?title=MediaWiki:Gadget-ListingEditor2023.js&action=raw&ctype=text/javascript');
It supports English and Italian for now. The fact that this gadget has been widely forked and altered is highly unfortunate as it means it's hard to switch over every language.
Pull requests welcome: https://github.com/jdlrobson/-Gadget-Workshop Jdlrobson (talk) 22:57, 14 July 2023 (UTC)[reply]
Retrieved from "https://en.wikivoyage.org/w/index.php?title=MediaWiki_talk:Gadget-ListingEditor.js&oldid=4717082"