1164353 - migrate browser context menu to use smart-modal-dialog

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Gaia::TV::System, defect)

Description

[John Hu [:johnhu][:johu][:醬糊小弟]]

migrate browser context menu to use smart-modal-dialog

Comment 1

[John Hu [:johnhu][:johu][:醬糊小弟]]

WIP can be found at: https://github.com/huchengtw-moz/gaia/commit/d7100a5c046578f441aeff62e860515a427e92a7

Comment 2

[GitHub Autolander Bot]

Attachment: [gaia] huchengtw-moz:bug-1164353-browser-context-menu-smart-modal-dialog > mozilla-b2g:master

Comment 3

[John Hu [:johnhu][:johu][:醬糊小弟]]

Comment on attachment 8607307 [details] [review]
[gaia] huchengtw-moz:bug-1164353-browser-context-menu-smart-modal-dialog > mozilla-b2g:master

Please review this patch. I had done the following changes:
1. use smart-modal-dialog to show the context menu
2. move/remove css rules to browser_context_menu.css

Comment 4

[KM Lee [:rexboy] (inactivate after Sep. 22)]

Comment on attachment 8607307 [details] [review]
[gaia] huchengtw-moz:bug-1164353-browser-context-menu-smart-modal-dialog > mozilla-b2g:master

Looks good to me with one comment on Github.

Comment 5

[Sung Chiu[:sungchiu]]

Comment on attachment 8607307 [details] [review]
[gaia] huchengtw-moz:bug-1164353-browser-context-menu-smart-modal-dialog > mozilla-b2g:master

Looks good to me!

Comment 6

[John Hu [:johnhu][:johu][:醬糊小弟]]

merged to master:
https://github.com/mozilla-b2g/gaia/commit/14968bb2c34ca406a696d8281aa0b76cf87dbbff


https://treeherder.mozilla.org/#/jobs?repo=gaia&revision=05e23fb2901e114e666a9fd1a2afd70af162acc6

Comment 7

[Tzu-Lin Huang [:dwi2][:tzhuang]]

backed out because this patch breaks context menu functionality in app-deck
https://github.com/mozilla-b2g/gaia/commit/fc14041b61966b630d129c89075f4b6d0357765d

Comment 8

[GitHub Autolander Bot]

Attachment: [gaia] huchengtw-moz:bug-1164353-v2 > mozilla-b2g:master

Comment 9

[John Hu [:johnhu][:johu][:醬糊小弟]]

Comment on attachment 8607933 [details] [review]
[gaia] huchengtw-moz:bug-1164353-v2 > mozilla-b2g:master

Sorry for the regression. I had changed the patch as following:
1. change the wrong CSS rules. This is the root cause of this issue. The default visibility should be hidden. Once we show the context menu, we created the context menu UI which is always visible and blocks all events.

2. remove useless code at hide().

Comment 10

[John Hu [:johnhu][:johu][:醬糊小弟]]

Comment on attachment 8607933 [details] [review]
[gaia] huchengtw-moz:bug-1164353-v2 > mozilla-b2g:master

We found a missing animation issue with this and previous patch. It is caused by lacking reflow. Once bug 1166673 landed, I will put review flag again. Thanks.

Comment 11

[John Hu [:johnhu][:johu][:醬糊小弟]]

Comment on attachment 8607933 [details] [review]
[gaia] huchengtw-moz:bug-1164353-v2 > mozilla-b2g:master

Sung,

I had revised the patch to include the latest version of smart-modal-dialog. Please review it again.

Comment 12

[Sung Chiu[:sungchiu]]

Comment on attachment 8607933 [details] [review]
[gaia] huchengtw-moz:bug-1164353-v2 > mozilla-b2g:master

Looks good to me!

Comment 13

[GitHub Autolander Bot]

https://github.com/mozilla-b2g/gaia/pull/30143

The pull request could not be applied to the integration branch. Please try again after current integration is complete. You may need to rebase your branch against the target branch.

Comment 14

[Kevin Grandon :kgrandon]

Tree closure causing is causing some weird issues. Let's land this manually for now, sorry for the inconvenience: https://github.com/mozilla-b2g/gaia/commit/f968fd179b51624c19021cb816b2e16d660abcdb