@Lydia_Pintscher @johl is it possible WMDE could help with this? Its a fairly major blocker to new contributors learning the basics of Wikidata

We'll have a look.

In T213704#4947316, @Lydia_Pintscher wrote:

We'll have a look.

Great, thanks, let me know how I can help :)

@Addshore and @Sebastian_Berlin-WMSE just wanted to flag you've both moved this task to your immediate to do lists in case you wanted to work together on it :)

The fact that I put it in my "This Week" initially means that I will investigate this; I haven't worked with tours before, so I need to figure out what's going on.

@Addshore, do you know how tours work and have time to work on this? If so, you're more qualified than me.

I looked into this and these are the easiest way to reproduce the problem:

1. Go to https://test.wikidata.org/w/index.php?title=Q1027&tour=wbtest&data=ok
2. Click on "Edit" as it's attached to it:

3. Click on "add qualifier" as it's attached to it:

The reason that seems most likely to me is the race condition between wikibase javascript trying to re-render the DOM and guided tour javascript trying to hook into new DOM. This is not easily fixable as they are completely different things but we can just introduce a little bit delay into hooking into new DOM in the guided tour library and it would be okay since javascript here doesn't make any API calls but if we are hooking into something that will wait for API response, we will hit the same issue again.
Let me run a test and make sure.

Yes, my guess was correct and it's a race condition. This hack fixes it: https://test.wikidata.org/w/index.php?title=MediaWiki:Guidedtour-lib.js&diff=470003&oldid=460451

Eww, race conditions. Would it be feasible to do something with MutationObserver to not have to use timers? That is if I understand it correctly, that the element we want to attach to hasn't been added yet when we want to attach it.

I did some experiments and I think this might work:

if($(step.attachTo).length === 0 {
  var observer = new MutationObserver(function (mutationList, observer) {
    for(var mutation of mutationList) {
      for(var node of mutation.addedNodes) {
        if(node === $(step.attachTo)) {
          e.stopPropagation();
          mw.libs.guiders.next();
        }
      }
    }
  });
  var _targetNode = $("body").get(0);
  var observerOptions = {
    childList: true,
    subtree: true
  }
  observer.observe(_targetNode, observerOptions);
} else {
  e.stopPropagation();
  mw.libs.guiders.next();
}

I don't have the rights to edit Guidedtour-lib.js; @Ladsgroup, can you try replacing the timeout with the above snippet and see if it works?

@Ladsgroup and @Sebastian_Berlin-WMSE did it work?

I'm still seeing boxes in the wrong place :(

My proposed code hasn't been added. Someone with the necessary user right needs to do this; I don't have those.

In T213704#5023783, @Sebastian_Berlin-WMSE wrote:

My proposed code hasn't been added. Someone with the necessary user right needs to do this; I don't have those.

Thanks for writing it :) Who has the power? How can we ask them nicely?

@MartinPoulter (saw your post about Wikidata on Commons), could that be turned into a tour?

@Sebastian_Berlin-WMSE I think I've found someone who could do a code review, but I don't know how to answer his question, see https://www.wikidata.org/wiki/Wikidata:Project_chat#Code_review_request_to_fix_Wikidata_Tours

@Addshore @Lydia_Pintscher is there a process for getting a code review for something working on beta Wikidata?

I'v got a bit lost in this ticket now.
What is the extra bit of patching that we have? Where is it? :)

It looks like perhaps this is already being solved in https://www.wikidata.org/wiki/Wikidata:Project_chat#Code_review_request_to_fix_Wikidata_Tours ?

@Sebastian_Berlin-WMSE I implemented your patch on test.wikidata but it does not seem to work. Do you know how to improve it?
Otherwise I think we can proceed with the hack by @Ladsgroup.

It's hard to work on this without being able to test the code myself. Is it possible to setup my common.js to run the tour code in a way that I can experiment there? I had a look at that, but couldn't figure out how to do it properly. Alternatively if I could have the rights to edit the actual tour script, that could work too.

In T213704#5068921, @Sebastian_Berlin-WMSE wrote:

It's hard to work on this without being able to test the code myself. Is it possible to setup my common.js to run the tour code in a way that I can experiment there? I had a look at that, but couldn't figure out how to do it properly. Alternatively if I could have the rights to edit the actual tour script, that could work too.

You can just put it in "User:Sebastian_Berlin-WMSE/common.js"

In T213704#5051741, @Pasleim wrote:

Otherwise I think we can proceed with the hack by @Ladsgroup.

The problem with this solution is that it's quite fragile: if the delay for the other operation is longer than the timeout, you will still get the unwanted behaviour. On the other hand, increasing the timeout to prevent this means that the user will experience a delay, even when not necessary.

The best solution I can think of is to use a callback for the other operation, but judging from previous comments, it seems that that code was to "far away" to do that.

In T213704#5068924, @Ladsgroup wrote:

In T213704#5068921, @Sebastian_Berlin-WMSE wrote:

It's hard to work on this without being able to test the code myself. Is it possible to setup my common.js to run the tour code in a way that I can experiment there? I had a look at that, but couldn't figure out how to do it properly. Alternatively if I could have the rights to edit the actual tour script, that could work too.

You can just put it in "User:Sebastian_Berlin-WMSE/common.js"

Will that override the original tours script? I thought it would just add it on top, which probably wouldn't turn out great.

@Ladsgroup @Pasleim @Addshore please can you help with trying to make this work? We are really dependent on this working so we can have tours to onboard new people, both in general and for our project FindingGLAMs (trying to create a worldwide database of cultural heritage institutions on Wikidata) https://meta.wikimedia.org/wiki/FindingGLAMs

Thanks very much

Hey, I really like to implement @Sebastian_Berlin-WMSE's fix in Wikidata but I can't test it and my knowledge of frontend is not good enough to understand it and its implications (for example if all of supported browsers support the implementation). If another developer gives a virtual +2, I'll happily put it in Wikidata

@Ladsgroup thanks very much, do you know who would be the right person/people to ask about frontend who could assess it?

Thanks again

I don't want to bother them but if they have time @Jakob_WMDE and @thiemowmde both have strong frontend knowledge.

Thanks @Ladsgroup , @Jakob_WMDE and @thiemowmde if there's any tasks I can swap you for or any other favours that would free up a bit of time to look at this please let me know (I'm a muggle with no programming skills)

Just to update this ticket:

As can be seen in https://test.wikidata.org/w/index.php?title=MediaWiki:Guidedtour-lib.js&action=history both the hack by @Ladsgroup and @Sebastian_Berlin-WMSE have been put on test.wikidata.org and "tested". Even with the latest code by @Sebastian_Berlin-WMSE the boxes are still not pinned to the right place.

I guess an ideal solution would be to make it clear how to work on the guided tour code without having to edit the "live version".
Second might be reverting to @Ladsgroup 's hack.

I didn't actually have any joy just pasting a copy of Guidedtour-lib.js into my common.js. Looking in &debug=true I suspect that my common.js is loaded and evaluated before the tour code.

Moved back to ready to go since it's no longer blocked by the investigation task but the problem still exists,

I believe we run into a very similar issue in the tour we have in the Two-Column-Edit-Conflict-Merge extension, see T195713: Have an intro tour (No. 2) as well as the patch https://gerrit.wikimedia.org/r/456176. The issue was that all the positions have been calculated to early, directly after the page finished loading. The moment the positions became relevant (i.e. the user clicked a button and the next popup was shown), all the other elements on the page could have been moved, i.e. when the user resized the browser window in the meantime, or a lazy-loading gadget made parts of the page reflow. I guess something similar might happen here. I know some quite relevant parts of the Wikibase UI (term box, collapsing references and qualifiers) only show up after the basic page loaded.

Personally, I'm not able to invest time on this, unless I'm instructed to do so.

In T213704#5135055, @thiemowmde wrote:

I believe we run into a very similar issue in the tour we have in the Two-Column-Edit-Conflict-Merge extension, see T195713: Have an intro tour (No. 2) as well as the patch https://gerrit.wikimedia.org/r/456176. The issue was that all the positions have been calculated to early, directly after the page finished loading. The moment the positions became relevant (i.e. the user clicked a button and the next popup was shown), all the other elements on the page could have been moved, i.e. when the user resized the browser window in the meantime, or a lazy-loading gadget made parts of the page reflow. I guess something similar might happen here. I know some quite relevant parts of the Wikibase UI (term box, collapsing references and qualifiers) only show up after the basic page loaded.

Personally, I'm not able to invest time on this, unless I'm instructed to do so.

Thanks very much @thiemowmde, just so I understand, you would be able to fix this issue if you were given it as a task by your line manager?

you would be able to fix this issue if you were given it as a task by your line manager?

It's open source software. People can change it and fix everything. The only question is how many resources people are willing to spend. In a worst-case scenario it might take months and a complete rewrite, but it's doable.

At this point my guess on how many resources this will require is at good as yours.

The discussions above suggest that the biggest issue with the current code is that it is not in a Git repository, but stuck on some wiki pages. I suggest to make it an actual MediaWiki extension and continue from there. But again, how many resources are available for such a project?

@thiemowmde, thanks, it seems like there are a few issues, please let me know if I missed something:

1. Technical knowledge needed to write the fix for the issue (one way or another)
2. The permissions to review the code and implement it from someone at Wikimedia DE (not sure who this is or if its more than one person?)
3. The whole thing may benefit from being rewritten as a Mediawiki extension (but this isn't required to fix the issue we've raised)

Is this broadly correct?

I'm somewhat confused what the relevance of these questions is, why they are asked, and who asks them. Are you a WMF or WMDE employee I'm not aware of? I don't know who "owns" the feature this ticket is about. It's on wikidata.org and falls into the Wikidata domain. But to me it looks more like the feature and the code behind it is a volunteer thing, and the volunteer Wikidata community would be the one who is responsible to maintain it. Permissions or technical knowledge have never been an issue, as far as I'm aware of. Both can be negotiated or build up if needed.

I don't get the confusion. John has explained why he is asking these questions: an important feature of Wikidata, used in an ongoing project, is not working properly and he's just trying to find out what steps are necessary to get it fixed. You've been asked because you've been recommended as someone with relevant knowledge. He's just looking to take the next step to get the problem fixed, whatever that step is. Discussing his employer doesn't identify that step.

Hi @Mrjohncummings and others. WMDE Wikidata engineers will try to help the problem with this Guided Tour.

To be clear about it: WMDE is not going to be able to drive overhauling GuidedTours gadget, should this become the part of the solution. This seems like a slightly bigger task, which is beyond our current capacities.
While what @Tarrow suggested above, to move the code to some other location that would allow for better maintainability is a great hint indeed, let's see if we could still help regardless.

@Mrjohncummings @Sebastian_Berlin-WMSE: forgive me my ignorance when talking about Guided Tours. I admit I have had very little experience with it, hence might be naming things wrong, or miss details which should have been obvious.

Given there has been quite some work around the issue, I'd like to take a step back and make sure I understand what is the current state, i.e. what is currently working wrong.

I've just opened https://test.wikidata.org/w/index.php?title=Q1027&tour=wbqualifiers&data=ok and while going through a tour, I noticed that the when on the step 8, ie. the one saying "In the new property field, start typing in "point in time" and select it from the drop-down to add.", the tour window is not attached to the newly opened input field, to which I should be typing into.
Windows related to prior steps, including Step 7's "Next click on the [add qualifier] button that appeared." are attached as expected.

Does this seem a correct observation and I pin pointed the issue right?

I've performed my tests using Firefox 66.0.3

@WMDE-leszek Thanks so much for taking a look at this!
Yes, the issue you describe on step 8 in your example is the main problem to solve. To me the root of the issue is that the user is allowed to skip through steps without having actually completed the instructions. This means that the popup is trying to attach to an element that is not visible on the page.

For this main issue, it seems that the only options are to either:

1. Complete each step for the user automatically if they have not done so themselves. This will insure that the relevant elements are visible at each step.

OR

2. Prevent the user from skipping to the next step until they have completed the current step (e.g. by disabling the next button)

Fixing with method 2 seems a lot simpler, albeit not as nice for the end user.

I would also note that there is another issue that has been identified as a 'race condition'. This is what some of the previous fixes posted earlier in the thread are addressing.
In this case, the element to attach to is visible but the somehow the popup is not still not attached. These are obviously connected but not the same problem. I have not experienced this second issue myself but others have reported it. I will do some further testing on this and post an example tomorrow with steps to reproduce.

Hopefully that makes sense, fire away with any further questions or things you's like us to test and report back on.

Many thanks!

Just to confirm from my previous message after some further testing, there are in fact two separate issues:

Bug 1 - User can reach a step where the tour popup needs to attach to an element which is not visible

If you use only the "next" arrow to get to the next tour window, then you can reach later steps without having actually completed the task in each instruction window.
If the skipped task was needed in order to show an element on screen for the next step, then the instruction window will default to the middle of the screen as the element it needs to attach to is not visible
Steps to reproduce:

1. Load this tour - https://test.wikidata.org/w/index.php?title=Q1027&tour=wbqualifiers&data=ok
2. On Step 6 "Add Qualifiers", do not complete the instruction to click on the "edit" button. Instead just click the "next" arrow.
3. On step 7, you can see that the tour popup is just in the centre of the screen. It is trying to attach to the "Add qualifier" button, but this is not visible because the "edit" section was not been opened in Step 6.

Bug 2 - Race between DOM loading and tour popup trying to attach to it

This only happens in two situations:

1. If the very first step in the tour has an attached popup

The page may not have been fully loaded when the Step 1 popup tries to attach to an element.
You can see an example of this when you load this test tour - https://test.wikidata.org/w/index.php?title=Q1027&tour=wbtest&data=ok
For me, the popup is not attached the first time the page loads (i.e. before it's cached). On subsequent reloads it works fine, but happens once again after I clear my cache

2. The user triggers the next step by clicking on a button which shows a new part of the page

This can happen in all the same situations as Bug 1 above, but by completing the instruction instead of skipping it with "next" arrow. So, whenever the user gets to the next step by performing an action that will show/create elements on the page, the next popup attachment needs to race with the DOM being updated (next popup is trying to attach to DOM elements that are being rendered simultaneously)
I can't repeat this issue reliably on any particular tour. Examples can be found in any tour steps that ask you to perform an action which shows new elements like "click 'edit' button", "click 'add qualifier' button", "click add labels". See Step 7 in this tour https://www.wikidata.org/w/index.php?title=Q16943273&tour=wbitems&data=ok

@WMDE-leszek if it is easier to have a call about this to discuss with myself and @NavinoEvans please let us know, we are both in the UK.

@Mrjohncummings apologies for the suboptimal response time. At WMDE we're in the middle of getting two bigger features through the door, which steals the most of my focus currently.
I will personally have a look at those two bugs at Wikimedia Hackathon this weekend. I hope I will get some of my WMDE colleagues into the fun two. I believe I understand the description you were kind to provide above.
Will any of you folks @Mrjohncummings @NavinoEvans @Sebastian_Berlin-WMSE happen to be in Prague this weekend? It would be fun to poke this thing together!

@WMDE-leszek amazing, thanks, how did you get on? Let me know if/how I can help

@WMDE-leszek this would be great feature to have working for Wikimania, we are having several GLAM workshops which include Wikidata and lack of good quality instructions is a real issue

@Mrjohncummings @NavinoEvans we've made some changes, which should hopefully help. I am not able to bring the changed files to test.wikidata.org due to lack of permissions. Would you mind try it yourselves? See the github gist linked T223999#5402050

Following up quickly the comment above: Per https://test.wikidata.org/wiki/Special:ListUsers?username=&group=interface-admin pinging @Pasleim @Ladsgroup @Rosalie_WMDE: maybe you folks could help with bring changes linked in https://phabricator.wikimedia.org/T223999#5402050 to test.wikidata.org ?

@Mrjohncummings @NavinoEvans The tool behaviour on test.wikidata.org https://test.wikidata.org/w/index.php?title=Q1027&tour=wbqualifiers2&data=ok.

Please note that for testing purposes the changes were added to wbqualifiers2 different from wbqualifiers.

@WMDE-leszek @Rosalie_WMDE Thank you so much for this, for me the issues are completely resolved in the test version :-)
@Mrjohncummings is that the same for you?

Thanks again @WMDE-leszek for picking this up, you've really saved the day. we have quite a few missing tours to create so will make good use of it once it goes live!

@WMDE-leszek @Rosalie_WMDE @NavinoEvans it works for me as well :) Thanks so much for making it happen :)

What needs to happen next to make it work on Wikidata?

All credit to @Rosalie_WMDE, she fixed it all! I was just a messenger.

@Mrjohncummings: to get this to Wikidata, one of interface administrators (https://www.wikidata.org/wiki/Special:ListUsers?username=&group=interface-admin) would need to update Wikidata's MediaWiki:Guidedtour-lib.js to match the one from test. And the tour definition itself, I reckon: MediaWiki:Guidedtour-tour-wbqualifiers.js
Or were there more files that you've been adapting on test.wikidata.org, @Rosalie_WMDE ?

In T213704#5415502, @WMDE-leszek wrote:

All credit to @Rosalie_WMDE, she fixed it all! I was just a messenger.

Thanks! I foward all the credit to the wikidata team. ;)

Or were there more files that you've been adapting on test.wikidata.org, @Rosalie_WMDE ?

No, you are right.

@Rosalie_WMDE thanks very much again,

@Ladsgroup could you press the magic buttons to make it work on Wikidata pretty please?

A couple months down the line, any movement here? Is this done?

Can we wait to let me make some more tours to test? I've tried some things and it worked fine but I feel like I really need to test it

thumbs up