Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Got JS for replace XBL tabmixbindings working! #3

Closed
wants to merge 66 commits into from

Conversation

117649
Copy link
Contributor

@117649 117649 commented Dec 31, 2020

This one is easier then I expect. Now the protect, locked and auto reload icon on tab is showing if you toggle the preference on and off and apply the changes to let the tab box got settings updated.

@117649
Copy link
Contributor Author

117649 commented Dec 31, 2020

protected, locked and auto reload icon on tab is showing now.

@117649
Copy link
Contributor Author

117649 commented Jan 4, 2021

I don't find any code that trigger the loading of dynamic styles. I can only apply the style by change prefs. Just wonder how this is done on old firefox @onemen .

@onemen
Copy link
Owner

onemen commented Jan 7, 2021

I don't find any code that trigger the loading of dynamic styles. I can only apply the style by change prefs. Just wonder how this is done on old firefox @onemen .

It is a little bit convoluted.

Most of dynamic style are triggers from modules/DynamicRules.jsm look below for the sequence that trigger it when ever new window is open.

setup.js

Tabmix.beforeBrowserInitOnLoad = function() {
  try {
    TabmixSvc.windowStartup.init(window);
  } catch (ex) {
    this.assert(ex);
  }

TabmixSvc.jsm

this.TabmixSvc = {
  ...
  windowStartup: {
      ...
      Cu.import("resource://tabmixplus/DynamicRules.jsm", tmp);
      tmp.DynamicRules.init(aWindow);
      ...    
  }
  ...
}

Some other rule can be found by searching for all the calls for gTMPprefObserver.insertRule or this.insertRule

@117649
Copy link
Contributor Author

117649 commented Jan 10, 2021

It looks like there is a problem with the progress bar on tab. The bar just can't get progress accurately only very small amount of pages can have their progress displayed others just have their aCurTotalProgress equals aMaxTotalProgress while the listener first ever been triggered.

@onemen
Copy link
Owner

onemen commented Jan 10, 2021

It looks like there is a problem with the progress bar on tab. The bar just can't get progress accurately only very small amount of pages can have their progress displayed others just have their aCurTotalProgress equals aMaxTotalProgress while the listener first ever been triggered.

it is better to drop this feature

@117649
Copy link
Contributor Author

117649 commented Jan 10, 2021

It looks like there is a problem with the progress bar on tab. The bar just can't get progress accurately only very small amount of pages can have their progress displayed others just have their aCurTotalProgress equals aMaxTotalProgress while the listener first ever been triggered.

it is better to drop this feature

Not necessary to drop it.
Even web extension add-on can provide basically same function. There is work around to be use.

@onemen
Copy link
Owner

onemen commented Jan 11, 2021

Please don't upload the file - tab_mix_plus-0.5.8.1-fx-p.xpi into the pull request.
can you i would like to add all your changes without the commits "Add files via upload".

Maybe I just squash all the the pull request into one commit

@117649
Copy link
Contributor Author

117649 commented Jan 12, 2021

I've added .gitignore
and for the progress bar on tab something like this:

let observer = new MutationObserver((mutations) => {
        mutations.forEach((mutation) => {
            mutation.addedNodes.forEach((node) => {
                if (node.nodeName == "BODY") {
                    inserted++;
                    node.addEventListener( "load", () => onLoadHandler(node), listenerCfg);
                    updateProgress();
                } else if (((node.nodeName == "SCRIPT" ||
                            node.nodeName == "VIDEO"  || node.nodeName == "IMG"    ||
                            node.nodeName == "IFRAME" || node.nodeName == "FRAME") && node.src != "" ) ||
                            (node.nodeName == "LINK" && node.rel == "stylesheet" && window.matchMedia(node.media))
                ) {
                    inserted++;
                    node.addEventListener( "error",   () => onLoadHandler(node), listenerCfg);
                    node.addEventListener( "abort",   () => onLoadHandler(node), listenerCfg);
                    node.addEventListener( "load",    () => onLoadHandler(node), listenerCfg);
                    updateProgress();
                }
            });
        });
    });

Would do the trick but I'm yet to find the way to insert it in to the content document.
Looks like I could just put this in content.js and have it send out messages.

@onemen
Copy link
Owner

onemen commented Jan 13, 2021

What will happen when a user with 100+ tabs loads all the tabs at startup ?

@117649
Copy link
Contributor Author

117649 commented Jan 13, 2021

What will happen when a user with 100+ tabs loads all the tabs at startup ?

I'm not yet have any success with the content.js the code is not as responsive as the web extension I'm still figuring out why.

@onemen
Copy link
Owner

onemen commented Jan 13, 2021

I'm currently working on merging your PR with my local repository, while applying lint rules.

I hope to push it to GitHub this week.

container: aWindow.document.createElement('box')
};

setAttribute(node.tempAppend.container, 'style', 'position: fixed; top: 4000px; left: 4000px; opacity: 0.001;');
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

'setAttribute' is not defined ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this one complain an error on you environment?


data.onBuild = function(aDocument, aDestroy) {
// Find the node in the DOM tree
var node = aDocument.getElementById(this.id);
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

'node' used outside of binding context.eslintblock-scoped-var
'node' is already declared in the upper scope.eslintno-shadow

node is argument of getWidgetData, use different var name here

// If it doesn't exist there either, CustomizableUI is using the widget information before it has been overlayed (i.e. opening a new window).
// We get a placeholder for it, then we'll replace it later when the window overlays.
if(!node && !aDestroy) {
var node = aDocument.importNode(Globals.widgets[this.id], true);
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same as above, it is also unclear if you want the node you declare here be the one that return by line 832

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well this whole block of code is actually chopped and pasted here from the Tab-group add-on for handle the toolbarbutton rightly. I didn't change any bit of it so it is sure to look quite weird.
But as long as I can tell they are working fine and not complain.

@onemen
Copy link
Owner

onemen commented Jan 14, 2021

Thank you @117649

I've just uploaded Tab mix complete history with your code merge to it.
You can close this PR #3 and #1.

I would appreciate if you can add a follow-up to PR #3 (in new PR) for the following:

  • remove commented code
  • remove non English comment
  • remove unused files that already replaced by Got JS for replace XBL tabmixbindings working! #3
    • xul files that replaced by xhtml files
    • xml files that replaced by jsm files, or js files (by the way what is CE stand for?)
    • css files ?
    • chrome.manifest - i don't know if this file still in use

For future patches pleas open an issue for each feature you are trying to fix and push the code for each issue to a separate pull request.
Make all pull request code to rewrite/main.

My Todo:

  • Use Prettier and Eslint to reformat all files (eslint recommended, prettier recommended).
  • Work on rewriting function calls that uses eval (there are 146), to modify Firefox code.
  • Rewrite options dialog to use in tab options like Firefox.
  • Remove obsolete code (code for compatibility with older versions of Firefox and/or Pale moon)

Let me know if there are any issue that you think I should prioritize

Since this rewrite is going to take a long time I think we should focus on makeing Tab mix compatible with latest Firefox Nightly and the next Firefox ESR. If compatibility with Waterfox will be straightforward we add it as well

@117649
Copy link
Contributor Author

117649 commented Jan 14, 2021

remove non English comment

The only one I know is in the prefs-ce.js don't know who created it but I'm sure it is converted from the old xbl bindings so the comment should just be the translation from the original I hope.

(by the way what is CE stand for?)

Custom Elements
I used this alias a lot because I copied prefs-ce.js from elsewhere and decided not to change the naming style, by no means this alias is formal.

chrome.manifest - i don't know if this file still in use

Yes they are used and almost function just like old time.
Except the 'overlay' and 'resource' instructions are powerless now.
Set 'overlay' in the bootstrap.js and replace 'resource' with 'content'.

So now there is only bootstrap way for legacy add-ons which means all things load and execute at a later time I already have to adjust the execute order of some code to get the tab appearance functions work.

For the Firefox versions compatibility it is a little bizarre situation here:

  1. For the latest Waterfox G3 which is based on Firefox 78ESR the compatibility should just be straightforward.

  2. You need a helper to load the bootstrap legacy add-on, Waterfox already came with one.

  3. The helper came in two types extension and script. I would consider the one waterfox provided is script type.

  4. The two types of the helper CAN co-exist BUT SHOULD NOT since this will cause two helper in the browser and load every bootstrap legacy add-on twice. Which will cause problem.

I hope you keep the preferences intact at least for the those related to tab appearances. Tab-group compatibility for Tabmix still lives don't want that to be ruined.
And I hope the progress bar on tab is kept they are such mentally comfortable to see. The 'onProgressChange' listener is not given right progress number every time now but 'onStateChange' is still good at least a pseudo progress is acceptable.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants