Introduce id props to tab and panel component in order to be selected by test

RESOLVED FIXED in Firefox 54

Status

()

Firefox
Developer Tools: Netmonitor
P1
normal
RESOLVED FIXED
a year ago
a year ago

People

(Reporter: rickychien, Assigned: rickychien)

Tracking

(Blocks: 1 bug)

unspecified
Firefox 54
Points:
---
Dependency tree / graph
Bug Flags:
qe-verify -

Firefox Tracking Flags

(firefox54 fixed)

Details

(Whiteboard: [netmonitor])

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(1 attachment)

We're using a dirty trick to select tab index (tab-0 ~ tab-6 and panel-0 ~ panel-6) for fixing all test failures in bug 1328197. It's not a good solution because tab order isn't predictable, so we should introduce an id for each tab and panel (#headers-tab, #headers-panel...etc) to make it selectable by test case.

We will make sure to handle mochitest failures and convert all of them to new id selector as well.
Blocks: 1307743
Flags: qe-verify?
Whiteboard: [netmonitor][triage] → [netmonitor] [triage]
Comment hidden (mozreview-request)
The patch adopted idea from tabbar component to introduce tab id (string type) for memorizing selected tab instead of index So, Redux state will memorize tab id as well.
Iteration: --- → 54.1 - Feb 6
Priority: -- → P1
Whiteboard: [netmonitor] [triage] → [netmonitor]
Flags: qe-verify? → qe-verify-
(Assignee)

Updated

a year ago
Blocks: 1333004
Comment hidden (mozreview-request)

Comment 4

a year ago
mozreview-review
Comment on attachment 8830129 [details]
Bug 1333364 - Introduce id props to tab

https://reviewboard.mozilla.org/r/107032/#review108496

Loks reasonable but, I am seeing one problem.

STR:

1) Open the side bar
2) Select a tab (different from the "Headers" selected by default.
3) Close/open the sidebar. The "Headers" tab is again selected by default -> BUG

Honza
Comment hidden (mozreview-request)
Thanks for reporting the issue. Patch has been updated for fixing the issue on comment 4.

Comment 7

a year ago
mozreview-review
Comment on attachment 8830129 [details]
Bug 1333364 - Introduce id props to tab

https://reviewboard.mozilla.org/r/107032/#review108528

The problem (default selected tab) is fixed.

Couple more comments.

Honza

::: devtools/client/netmonitor/events.js
(Diff revision 3)
>    // When the image response thumbnail is displayed in the UI.
>    RESPONSE_IMAGE_THUMBNAIL_DISPLAYED:
>      "NetMonitor:ResponseImageThumbnailAvailable",
>  
> -  // When a tab is selected in the NetworkDetailsView and subsequently rendered.
> -  TAB_UPDATED: "NetMonitor:TabUpdated",

I am still seeing a test where TAB_UPDATED is used.

browser_net_simple-request-details.js:

::: devtools/client/netmonitor/test/browser_net_security-icon-click.js:54
(Diff revision 3)
>  
> -    let wait = waitForDOM(document, "#panel-5");
> +    let wait = waitForDOM(document, "#security-panel");
>      info("Clicking security icon of the first request and waiting for panel update.");
>      EventUtils.synthesizeMouseAtCenter(icon, {}, monitor.panelWin);
>      yield wait;
> -    ok(document.querySelector("#tab-5.is-active"), "Security tab is selected.");
> +    debugger

debugger?

Comment 8

a year ago
mozreview-review
Comment on attachment 8830129 [details]
Bug 1333364 - Introduce id props to tab

https://reviewboard.mozilla.org/r/107032/#review108578
Attachment #8830129 - Flags: review?(odvarko)
(Assignee)

Comment 9

a year ago
mozreview-review-reply
Comment on attachment 8830129 [details]
Bug 1333364 - Introduce id props to tab

https://reviewboard.mozilla.org/r/107032/#review108528

> I am still seeing a test where TAB_UPDATED is used.
> 
> browser_net_simple-request-details.js:

Yes, browser_net_simple-request-details.js is disable (see test/browser.ini) and will be re-enabled in bug 1258809.
I'd prefer to keep it as is and fix in bug 1258809.
Comment hidden (mozreview-request)

Comment 11

a year ago
mozreview-review
Comment on attachment 8830129 [details]
Bug 1333364 - Introduce id props to tab

https://reviewboard.mozilla.org/r/107032/#review108614

Looks good to me now!

Thanks Ricky,
Honza
Attachment #8830129 - Flags: review?(odvarko) → review+

Comment 12

a year ago
Pushed by rchien@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/0301d66494e6
Introduce id props to tab r=Honza
Backed out for failing devtools/client/shared/components/test/mochitest/test_tabs_accessibility.html:

https://hg.mozilla.org/integration/autoland/rev/ee4ed7cdb771e19f418b5b424806bf97b90680fb

Push with failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=0301d66494e60f93d55d372aaf00fccf6b7b72bc
Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=72257326&repo=autoland

[task 2017-01-26T16:46:14.582713Z] 16:46:14     INFO - TEST-START | devtools/client/shared/components/test/mochitest/test_tabs_accessibility.html
[task 2017-01-26T16:46:14.855607Z] 16:46:14     INFO - TEST-INFO | started process screentopng
[task 2017-01-26T16:46:15.306359Z] 16:46:15     INFO - TEST-INFO | screentopng: exit 0
[task 2017-01-26T16:46:15.313106Z] 16:46:15     INFO - Buffered messages logged at 16:46:14
[task 2017-01-26T16:46:15.314944Z] 16:46:15     INFO - TEST-PASS | devtools/client/shared/components/test/mochitest/test_tabs_accessibility.html | li role is set correctly 
[task 2017-01-26T16:46:15.316248Z] 16:46:15     INFO - TEST-PASS | devtools/client/shared/components/test/mochitest/test_tabs_accessibility.html | Anchor role is set correctly 
[task 2017-01-26T16:46:15.317417Z] 16:46:15     INFO - TEST-PASS | devtools/client/shared/components/test/mochitest/test_tabs_accessibility.html | Anchor aria-selected is set correctly by default 
[task 2017-01-26T16:46:15.318897Z] 16:46:15     INFO - Buffered messages finished
[task 2017-01-26T16:46:15.320613Z] 16:46:15     INFO - TEST-UNEXPECTED-FAIL | devtools/client/shared/components/test/mochitest/test_tabs_accessibility.html | Anchor aria-controls is set correctly - got "sidebar-panel-0-panel", expected "panel-0"
[task 2017-01-26T16:46:15.321641Z] 16:46:15     INFO - SimpleTest.is@chrome://mochikit/content/tests/SimpleTest/SimpleTest.js:271:5
[task 2017-01-26T16:46:15.322623Z] 16:46:15     INFO - window.onload<@chrome://mochitests/content/chrome/devtools/client/shared/components/test/mochitest/test_tabs_accessibility.html:43:5
[task 2017-01-26T16:46:15.323673Z] 16:46:15     INFO - _run@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/shared/task.js:311:39
[task 2017-01-26T16:46:15.324534Z] 16:46:15     INFO - promise callback*_handleResultValue@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/shared/task.js:387:7
[task 2017-01-26T16:46:15.325593Z] 16:46:15     INFO - _run@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/shared/task.js:319:13
[task 2017-01-26T16:46:15.326724Z] 16:46:15     INFO - promise callback*_handleResultValue@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/shared/task.js:387:7
[task 2017-01-26T16:46:15.327758Z] 16:46:15     INFO - _run@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/shared/task.js:319:13
[task 2017-01-26T16:46:15.328826Z] 16:46:15     INFO - EventHandlerNonNull*@chrome://mochitests/content/chrome/devtools/client/shared/components/test/mochitest/test_tabs_accessibility.html:19:17
[task 2017-01-26T16:46:15.330270Z] 16:46:15     INFO - TEST-PASS | devtools/client/shared/components/test/mochitest/test_tabs_accessibility.html | li role is set correctly 
[task 2017-01-26T16:46:15.331430Z] 16:46:15     INFO - TEST-PASS | devtools/client/shared/components/test/mochitest/test_tabs_accessibility.html | Anchor role is set correctly 
[task 2017-01-26T16:46:15.332666Z] 16:46:15     INFO - TEST-PASS | devtools/client/shared/components/test/mochitest/test_tabs_accessibility.html | Anchor aria-selected is set correctly by default 
[task 2017-01-26T16:46:15.333915Z] 16:46:15     INFO - Not taking screenshot here: see the one that was previously logged
[task 2017-01-26T16:46:15.335469Z] 16:46:15     INFO - TEST-UNEXPECTED-FAIL | devtools/client/shared/components/test/mochitest/test_tabs_accessibility.html | Anchor aria-controls is set correctly - got "sidebar-panel-1-panel", expected "panel-1"
[task 2017-01-26T16:46:15.336343Z] 16:46:15     INFO - SimpleTest.is@chrome://mochikit/content/tests/SimpleTest/SimpleTest.js:271:5
[task 2017-01-26T16:46:15.337327Z] 16:46:15     INFO - window.onload<@chrome://mochitests/content/chrome/devtools/client/shared/components/test/mochitest/test_tabs_accessibility.html:47:5
[task 2017-01-26T16:46:15.338098Z] 16:46:15     INFO - _run@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/shared/task.js:311:39
[task 2017-01-26T16:46:15.338857Z] 16:46:15     INFO - promise callback*_handleResultValue@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/shared/task.js:387:7
[task 2017-01-26T16:46:15.339548Z] 16:46:15     INFO - _run@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/shared/task.js:319:13
[task 2017-01-26T16:46:15.340329Z] 16:46:15     INFO - promise callback*_handleResultValue@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/shared/task.js:387:7
[task 2017-01-26T16:46:15.341072Z] 16:46:15     INFO - _run@resource://gre/modules/commonjs/toolkit/loader.js -> resource://devtools/shared/task.js:319:13
[task 2017-01-26T16:46:15.341893Z] 16:46:15     INFO - EventHandlerNonNull*@chrome://mochitests/content/chrome/devtools/client/shared/components/test/mochitest/test_tabs_accessibility.html:19:17
Flags: needinfo?(rchien)
Comment hidden (mozreview-request)
a11y test failures are fixed
Flags: needinfo?(rchien)

Comment 16

a year ago
Pushed by rchien@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/4be138bdb7bf
Introduce id props to tab r=Honza

Comment 17

a year ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/4be138bdb7bf
Status: ASSIGNED → RESOLVED
Last Resolved: a year ago
status-firefox54: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 54
You need to log in before you can comment on or make changes to this bug.