Closed Bug 1333364 Opened 8 years ago Closed 8 years ago

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

Categories

(DevTools :: Netmonitor, defect, P1)

Product:
DevTools
Component:
Netmonitor
Type:
defect

Tracking

(firefox54 fixed)

Status:
RESOLVED FIXED
Milestone:
Firefox 54
Iteration:
54.1 - Feb 6
Tracking Flags:
Tracking Status
firefox54 --- fixed

People

(Reporter: rickychien, Assigned: rickychien)

References

(Blocks 1 open bug)

Details

(Whiteboard: [netmonitor])

Attachments

(1 file)

Bug 1333364 - Introduce id props to tab
59 bytes, text/x-review-board-request
Honza
: review+
Details
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: netmonitor-html
Flags: qe-verify?
Whiteboard: [netmonitor][triage] → [netmonitor] [triage]
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-
Blocks: 1333004
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
Thanks for reporting the issue. Patch has been updated for fixing the issue on comment 4.
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 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 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+
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)
a11y test failures are fixed
Flags: needinfo?(rchien)
https://hg.mozilla.org/mozilla-central/rev/4be138bdb7bf
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
status-firefox54: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 54
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: