Dynamic tabs: use buttons rather than links (backport to v4)

[patrickhlauke]

Manual backport of #32630

[patrickhlauke]

Not sure why the tests are failing ... looks like some connection problem / not spinning up Chrome 88 headless browser instance?

[XhmikosR]

Something is wrong with the tests code changes, not sure what yet.

EDIT: try using var done = assert.async() and call done() when you expect a test to be finished.

[patrickhlauke]

EDIT: try using var done = assert.async() and call done() when you expect a test to be finished.

apart from adding one test (literally copying it from the previous one, but with different document to test on) and making minor tweaks to the others, i've not done anything new test-wise. wonder why it's now failing when before it was working fine without any tweaks necessary?

[patrickhlauke]

I'll need a little assist from somebody who knows how the tests stuff actually works (and why all of a sudden it doesn't anymore despite what I thought were just minimal changes ... I probably broke something along the way, but don't know enough about qUnit to work out what exactly). @rohit2sharma95 perhaps?

[patrickhlauke]

not sure if this of any help, but even trying to run the tests locally ends up spewing up errors

02 03 2021 21:47:38.029:ERROR [karma-server]: Server start failed on port 9876: Error: Please install Chrome, Chromium or Firefox
02 03 2021 21:47:38.170:ERROR [karma-server]: Server start failed on port 9877: Error: Please install Chrome, Chromium or Firefox

Not sure if this is something more than just the few tests I've tweaked...could it be a more fundamental change/problem with karma or something?

karma-runner/karma-chrome-launcher#154

[patrickhlauke]

FWIW I get karma failures (locally) even when trying to run npm test on the main v4-dev branch...

[ffoodd]

@patrickhlauke I guess you have Chrome or Firefox installed?
@XhmikosR could that relate to the latest change with our karma config that I had to try on Ubuntu?

[rohit2sharma95]

FWIW I get karma failures (locally) even when trying to run npm test on the main v4-dev branch...

I am also facing the same issue and it is not this PR specific. 🤔

[XhmikosR]

I don't think it's related at least it works fine for me here with Chrome on Windows 10. The test failures are specific to this branch for me, like on CI. I can't comment for other OS'es. Unfortunately, we don't seem to have a debug script on v4-dev. And I cannot pinpoint which test file is causing the hang.

[XhmikosR]

I believe the core JS code doesn't support the new button markup. Or the markup used is wrong.

[XhmikosR]

OK, NVM the above comment. The added code is not right for sure.

1. should not fire shown when tab is disabled and show and shown events should reference correct relatedTarget are broken so I had to use QUnit.skip to temporarily run the tests

2.  Chrome Headless 88.0.4324.190 (Windows 10) tabs selected tab should have aria-selected FAILED
            hidden tab has aria-selected = false
            Expected: "false"
            Actual: undefined
                at Object.<anonymous> (js/tests/unit/tab.js:316:12)
                at runTest (node_modules/qunit/qunit/qunit.js:2251:36)
                at Test.run (node_modules/qunit/qunit/qunit.js:2239:8)
                at node_modules/qunit/qunit/qunit.js:2461:15
                at processTaskQueue (node_modules/qunit/qunit/qunit.js:1849:32)
                at node_modules/qunit/qunit/qunit.js:1853:12
   
            after click, hidden tab has aria-selected = false
            Expected: "false"
            Actual: undefined
                at Object.<anonymous> (js/tests/unit/tab.js:320:12)
                at runTest (node_modules/qunit/qunit/qunit.js:2251:36)
                at Test.run (node_modules/qunit/qunit/qunit.js:2239:8)
                at node_modules/qunit/qunit/qunit.js:2461:15
                at processTaskQueue (node_modules/qunit/qunit/qunit.js:1849:32)
                at node_modules/qunit/qunit/qunit.js:1853:12
   
            hidden tab has aria-selected = false
            Expected: "false"
            Actual: undefined
                at Object.<anonymous> (js/tests/unit/tab.js:324:12)
                at runTest (node_modules/qunit/qunit/qunit.js:2251:36)
                at Test.run (node_modules/qunit/qunit/qunit.js:2239:8)
                at node_modules/qunit/qunit/qunit.js:2461:15
                at processTaskQueue (node_modules/qunit/qunit/qunit.js:1849:32)
                at node_modules/qunit/qunit/qunit.js:1853:12
   
            after second show event, hidden tab has aria-selected = false
            Expected: "false"
            Actual: undefined
                at Object.<anonymous> (js/tests/unit/tab.js:328:12)
                at runTest (node_modules/qunit/qunit/qunit.js:2251:36)
                at Test.run (node_modules/qunit/qunit/qunit.js:2239:8)
                at node_modules/qunit/qunit/qunit.js:2461:15
                at processTaskQueue (node_modules/qunit/qunit/qunit.js:1849:32)
                at node_modules/qunit/qunit/qunit.js:1853:12
   

[patrickhlauke]

@patrickhlauke I guess you have Chrome or Firefox installed?

i was going to say "duh, of course" ... but then mulling this over further, my command line runs in WSL under windows, and no in the ubuntu subsystem that this runs, I didn't have any browsers installed. urgh. but apparently it's not non-trivial to do, so leaving this for another night...

[patrickhlauke]

that tweak in tab.js was actually missing in v5 - added it here #33257

[patrickhlauke]

If anybody has the ability to work out what's going on here ... feel free to have a crack at it. Otherwise, let's just scrap this backport.

[alpadev]

@patrickhlauke tada 😊

[patrickhlauke]

Sorry, after leaving this dormant for so long, I think this is probably now as good as it gets for merging, if we still want to.