1583961 - Merge editMenuOverlay.js into globalOverlay.js

Status: NEW

(Toolkit :: UI Widgets, task, P5)

Description

[Brian Grinstead [:bgrins]]

Something we've noticed while converting <textbox> to <input> (bug 1547372) is that we need to load both globalOverlay.js and editMenuOverlay.js in any document that wants context menu support. That's fine, but I don't know that it really makes sense to keep these separate because it's one extra chrome script to load at startup in the browser document, and one more extra thing we have to do for many chrome documents. editMenuOverlay.js is relatively side effect free other than:

1. adding the context menu listener which returns if it's not fired on <input>
2. injecting "editMenuCommands" DOM into the document https://searchfox.org/mozilla-central/rev/f43ae7e1c43a4a940b658381157a6ea6c5a185c1/toolkit/content/editMenuOverlay.js#43). I believe this could be lazified more by only adding when calling one of the goUpdate* functions in the file.

Comment 1

[Brian Grinstead [:bgrins]]

The idea here would be:

1. Remove all script tags / subscript loading for https://searchfox.org/mozilla-central/search?q=editmenuoverlay.js&case=false&regexp=false&path= (ensuring that globalOverlay.js is also loaded in each doc which it should be)
2. Copy contents of https://searchfox.org/mozilla-central/source/toolkit/content/editMenuOverlay.js into https://searchfox.org/mozilla-central/source/toolkit/content/globalOverlay.js
3. Remove the file and the reference in jar.mn: https://searchfox.org/mozilla-central/rev/f43ae7e1c43a4a940b658381157a6ea6c5a185c1/toolkit/content/jar.mn#44.

Gijs, do you have an opinion on if we should do this? I've been thinking about it for a while and want to get a second opinion as to if this is a good idea.

Comment 2

[:Gijs (he/him)]

(In reply to Brian Grinstead [:bgrins] from comment #1)

The idea here would be:

1. Remove all script tags / subscript loading for https://searchfox.org/mozilla-central/search?q=editmenuoverlay.js&case=false&regexp=false&path= (ensuring that globalOverlay.js is also loaded in each doc which it should be)

Looks like it is for all of the hits I see (though it's late so doublecheck me, maybe I missed something).

2. Copy contents of https://searchfox.org/mozilla-central/source/toolkit/content/editMenuOverlay.js into https://searchfox.org/mozilla-central/source/toolkit/content/globalOverlay.js
3. Remove the file and the reference in jar.mn: https://searchfox.org/mozilla-central/rev/f43ae7e1c43a4a940b658381157a6ea6c5a185c1/toolkit/content/jar.mn#44.

Gijs, do you have an opinion on if we should do this? I've been thinking about it for a while and want to get a second opinion as to if this is a good idea.

Makes sense to me. I'll add two quick notes:

• https://bugzilla.mozilla.org/show_bug.cgi?id=1497200 just touched this, so if you write a patch make sure it's on top of that or you'll just have rebase pains
• I think there may be a missing step, which is ensuring that all the things that load globalOverlay and don't currently load editMenuOverlay will be happy with this change (no conflicting variables + no upset at the behaviour that the edit stuff introduces)