Closed Bug 1620098 Opened 5 years ago Closed 4 years ago

Can't access all items in drop down list of device sizes

Categories

(DevTools :: Responsive Design Mode, defect, P1)

Product:
DevTools
Component:
Responsive Design Mode
Version:
75 Branch
Type:
defect

Tracking

(firefox-esr68 unaffected, firefox74 unaffected, firefox75 wontfix, firefox76 wontfix, firefox77 wontfix, firefox78 verified)

Status:
VERIFIED FIXED
Milestone:
Firefox 78
Tracking Flags:
Tracking Status
firefox-esr68 --- unaffected
firefox74 --- unaffected
firefox75 --- wontfix
firefox76 --- wontfix
firefox77 --- wontfix
firefox78 --- verified

People

(Reporter: shobson, Assigned: daisuke)

References

(Regressed 1 open bug, Regression)

Details

(Keywords: regression)

Attachments

(4 files, 1 obsolete file)

Screen Shot 2020-03-04 at 15.47.43.png
175.55 KB, image/png
Details
Bug 1620098: Show HTMLTooltip in html document as XUL popup.
47 bytes, text/x-phabricator-request
Details | Review
Bug 1620098: Remove unnecessary processing in RDM.
47 bytes, text/x-phabricator-request
Details | Review
Bug 1620098: Avoid calling onUpdateDeviceModal twice when closing the device modal.
47 bytes, text/x-phabricator-request
Details | Review

Thank you for helping make Firefox better. If you are reporting a defect, please complete the following:

What were you doing?

Using Responsive Design Mode and the HTML inspector docked to the bottom of my screen on a laptop. In Nightly.

  1. Open RDM
  2. Open HTML Inspector
  3. Make HTML Inspector really tall
  4. Try to change the device in the RDM device drop down

What happened?

I can only access items at the bottom of the drop down.

What should have happened?

Probably I should be able to scroll the list if it doesn't all fit in the available space?

Daisuke, I think you worked on the dropdown menu recently. Would you know the cause for this? Thanks!

Flags: needinfo?(daisuke)

Thank you for letting me know!
I also could reproduce this.

I suppose the reason is that we started to use HTML for the menu instead of XUL.
I will take a look.

Comment on attachment 9131403 [details]
Bug 1620098: Change the container of the popup in order to use XUL element.

Hello Gabriel and Micah!

As I commented, the reason why the tooltip can't overlay the inspector pane is because of using HTML for the menu.
Thus, in order to overlay, it seems we can do if we use XUL element and prepare a place where holds the element outside of the responsive document.
I made a prototype with the above thinking and upload the patch.
Could you give your opinion about whether this approach is good or not?

Flags: needinfo?(daisuke)
Attachment #9131403 - Flags: feedback?(tigleym)
Attachment #9131403 - Flags: feedback?(gl)

Ah, I did not apply the style yet for the prototype.

Comment on attachment 9131403 [details]
Bug 1620098: Change the container of the popup in order to use XUL element.

I am gonna also request feedback from Julian since he also has a bit more context on this with the work he has done with HTMLTooltips.

Attachment #9131403 - Flags: feedback?(jdescottes)

Thanks for the ping!
I think we can simplify the approach, added a comment in the diff.

Daisuke, did you notice that if you:

  • open the menu
  • click on the menu button again

The menu disappears and re-appears immediately, at the moment it simply closes.

Comment on attachment 9131403 [details]
Bug 1620098: Change the container of the popup in order to use XUL element.

f+ if we can handle this by changing the isXUL logic.

Attachment #9131403 - Flags: feedback?(jdescottes) → feedback+

Thank you very much for your feedback and helpful advice always, Julian!
I'll take a look and fix the behavior and the isXUL logic!

Comment on attachment 9131403 [details]
Bug 1620098: Change the container of the popup in order to use XUL element.

I think we're in good hands here after bgrins and jdescottes took a look through it so I will remove the feedback for myself.

Attachment #9131403 - Flags: feedback?(gl)

The priority flag is not set for this bug.
:gl, could you have a look please?

For more information, please visit auto_nag documentation.

Flags: needinfo?(gl)
Flags: needinfo?(gl)
Priority: -- → P1

Depends on D67983

Assignee: nobody → daisuke
Status: NEW → ASSIGNED

Depends on D67984

Copying flags over from bug 1624361

status-firefox74: --- → unaffected
status-firefox75: --- → affected
status-firefox76: --- → affected
status-firefox-esr68: --- → unaffected
Keywords: regression
Regressed by: 1610594
Has Regression Range: --- → yes
Has STR: --- → yes
Depends on: 1629770

Hi Daisuke, are you still actively working on this one?

Flags: needinfo?(daisuke)

Hi Rachel!
Yes, I am. However, as this bug is dependent on bug 1629770, I'm waiting until fixing that.

Flags: needinfo?(daisuke)
Pushed by dakatsuka.birchill@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/6b397048940f
Show HTMLTooltip in html document as XUL popup. r=bgrins,jdescottes,mtigley,yzen,bdahl
https://hg.mozilla.org/integration/autoland/rev/2996bd22bd97
Remove unnecessary processing in RDM. r=mtigley
https://hg.mozilla.org/integration/autoland/rev/a3a8244860bc
Avoid calling onUpdateDeviceModal twice when closing the device modal. r=mtigley

Push with failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&resultStatus=testfailed%2Cbusted%2Cexception&revision=a3a8244860bc2f4f2ac211db1200c34cda5b9129&selectedTaskRun=Q2UMSdzdQ0q1g5GdVd6irQ-1

Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=301139091&repo=autoland

Backout link: https://hg.mozilla.org/integration/autoland/rev/e392e4a34033d5b594da33bdbf207119bdd5af54

[task 2020-05-07T02:34:15.893Z] PASS components/badge.test.js
[task 2020-05-07T02:34:15.893Z]   Badge component:
[task 2020-05-07T02:34:15.893Z]     βœ“ basic render (29ms)
[task 2020-05-07T02:34:15.893Z] 
[task 2020-05-07T02:34:15.893Z] Test Suites: 2 failed, 12 passed, 14 total
[task 2020-05-07T02:34:15.893Z] Tests:       11 failed, 40 passed, 51 total
[task 2020-05-07T02:34:15.893Z] Snapshots:   48 passed, 48 total
[task 2020-05-07T02:34:15.893Z] Time:        13.17s
[task 2020-05-07T02:34:15.893Z] Ran all test suites.
[task 2020-05-07T02:34:15.893Z] error Command failed with exit code 1.
[task 2020-05-07T02:34:15.893Z] 
[task 2020-05-07T02:34:15.893Z] [devtools-node-test-runner] Parse errors from the test logs
[task 2020-05-07T02:34:15.893Z] TEST-UNEXPECTED-FAIL | jest | accessibility |   ● AccessibilityTreeFilter component: β€Ί audit filter not filtered
[task 2020-05-07T02:34:15.893Z] TEST-UNEXPECTED-FAIL | jest | accessibility |   ● AccessibilityTreeFilter component: β€Ί audit filters filtered
[task 2020-05-07T02:34:15.893Z] TEST-UNEXPECTED-FAIL | jest | accessibility |   ● AccessibilityTreeFilter component: β€Ί audit all filter not filtered auditing
[task 2020-05-07T02:34:15.893Z] TEST-UNEXPECTED-FAIL | jest | accessibility |   ● AccessibilityTreeFilter component: β€Ί audit other filter not filtered auditing
[task 2020-05-07T02:34:15.893Z] TEST-UNEXPECTED-FAIL | jest | accessibility |   ● AccessibilityTreeFilter component: β€Ί audit all filter filtered auditing
[task 2020-05-07T02:34:15.893Z] TEST-UNEXPECTED-FAIL | jest | accessibility |   ● AccessibilityTreeFilter component: β€Ί audit other filter filtered auditing
[task 2020-05-07T02:34:15.893Z] TEST-UNEXPECTED-FAIL | jest | accessibility |   ● AccessibilityTreeFilter component: β€Ί toggle filter
[task 2020-05-07T02:34:15.893Z] TEST-UNEXPECTED-FAIL | jest | accessibility |   ● AccessibilityTreeFilter component: β€Ί render filters after state changes
[task 2020-05-07T02:34:15.893Z] TEST-UNEXPECTED-FAIL | jest | accessibility |   ● AccessibilityPrefs component: β€Ί prefs not set by default
[task 2020-05-07T02:34:15.893Z] TEST-UNEXPECTED-FAIL | jest | accessibility |   ● AccessibilityPrefs component: β€Ί prefs checked
[task 2020-05-07T02:34:15.893Z] TEST-UNEXPECTED-FAIL | jest | accessibility |   ● AccessibilityPrefs component: β€Ί toggle pref
[task 2020-05-07T02:34:15.893Z] [devtools-node-test-runner] Test suite [accessibility] failed
[task 2020-05-07T02:34:15.893Z] [devtools-node-test-runner] You can find documentation about the devtools node tests at https://docs.firefox-dev.tools/tests/node-tests.html
[task 2020-05-07T02:34:15.893Z] ules/react-dom/cjs/react-dom.development.js:17258:5)
[task 2020-05-07T02:34:15.893Z]         at Object.render (/builds/worker/checkouts/gecko/devtools/client/accessibility/test/node/node_modules/react-dom/cjs/react-dom.development.js:17317:12)
[task 2020-05-07T02:34:15.894Z]         at render (/builds/worker/checkouts/gecko/devtools/client/accessibility/test/node/node_modules/enzyme-adapter-react-16/src/ReactSixteenAdapter.js:429:26)
[task 2020-05-07T02:34:15.894Z]         at fn (/builds/worker/checkouts/gecko/devtools/client/accessibility/test/node/node_modules/enzyme-adapter-react-16/src/ReactSixteenAdapter.js:343:12)
[task 2020-05-07T02:34:15.894Z]         at Object.wrapAct [as render] (/builds/worker/checkouts/gecko/devtools/client/accessibility/test/node/node_modules/enzyme-adapter-react-16/src/ReactSixteenAdapter.js:415:16)
[task 2020-05-07T02:34:15.894Z]         at new render (/builds/worker/checkouts/gecko/devtools/client/accessibility/test/node/node_modules/enzyme/src/ReactWrapper.js:115:16)
[task 2020-05-07T02:34:15.894Z]         at mount (/builds/worker/checkouts/gecko/devtools/client/accessibility/test/node/node_modules/enzyme/src/mount.js:10:10)
[task 2020-05-07T02:34:15.894Z]         at Object.mount (/builds/worker/checkouts/gecko/devtools/client/accessibility/test/node/components/accessibility-tree-filter.test.js:245:21)
[task 2020-05-07T02:34:15.894Z]         at Object.asyncJestTest (/builds/worker/checkouts/gecko/devtools/client/accessibility/test/node/node_modules/jest-jasmine2/build/jasmineAsyncInstall.js:102:37)
[task 2020-05-07T02:34:15.894Z]         at resolve (/builds/worker/checkouts/gecko/devtools/client/accessibility/test/node/node_modules/jest-jasmine2/build/queueRunner.js:43:12)
[task 2020-05-07T02:34:15.894Z]         at new Promise (<anonymous>)
[task 2020-05-07T02:34:15.894Z]         at mapper (/builds/worker/checkouts/gecko/devtools/client/accessibility/test/node/node_modules/jest-jasmine2/build/queueRunner.js:26:19)
[task 2020-05-07T02:34:15.894Z]         at promise.then (/builds/worker/checkouts/gecko/devtools/client/accessibility/test/node/node_modules/jest-jasmine2/build/queueRunner.js:73:41)
[task 2020-05-07T02:34:15.894Z]         at process._tickCallback (internal/process/next_tick.js:68:7)
[task 2020-05-07T02:34:15.894Z] 
[task 2020-05-07T02:34:15.894Z]   console.error node_modules/react-dom/cjs/react-dom.development.js:14226
Flags: needinfo?(daisuke)
Backout by csabou@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/109d3914c942
Backed out 3 changesets for devtools jtest failures. CLOSED TREE
Flags: needinfo?(daisuke)
Attachment #9131403 - Attachment is obsolete: true
Attachment #9131403 - Flags: feedback?(tigleym)
Pushed by dakatsuka.birchill@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/4c1410654ab7
Show HTMLTooltip in html document as XUL popup. r=bgrins,jdescottes,mtigley,yzen,bdahl,nchevobbe
https://hg.mozilla.org/integration/autoland/rev/fea2b74b3352
Remove unnecessary processing in RDM. r=mtigley
https://hg.mozilla.org/integration/autoland/rev/bd70a801f80c
Avoid calling onUpdateDeviceModal twice when closing the device modal. r=mtigley

https://hg.mozilla.org/mozilla-central/rev/4c1410654ab7
https://hg.mozilla.org/mozilla-central/rev/fea2b74b3352
https://hg.mozilla.org/mozilla-central/rev/bd70a801f80c

Status: ASSIGNED → RESOLVED
Closed: 4 years ago
status-firefox78: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 78

The patch landed in nightly and beta is affected.
:daisuke, is this bug important enough to require an uplift?
If not please set status_beta to wontfix.

For more information, please visit auto_nag documentation.

Flags: needinfo?(daisuke)
Flags: qe-verify+

Reproduced the issue using Firefox 75.0a1 (20200304161940) on Windows 10x64.
The issue is verified fixed with Firefox 78.0b7 (20200612174529) on Windows 10x64, macOS 10.12 and Ubuntu 18.04.

Status: RESOLVED → VERIFIED
status-firefox78: fixed → verified
Flags: qe-verify+
Regressions: 1666331
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: