windingwind / zotero-plugin-toolkit

Toolkit for Zotero Plugin Developers.
https://www.npmjs.com/package/zotero-plugin-toolkit
MIT License
104 stars 13 forks source link

registering prefpane fails #14

Closed retorquere closed 10 months ago

retorquere commented 1 year ago

I'm using the toolkit to load this prefpane: https://gist.github.com/988368e73613c8f3a44240afa7854ecf

but that gets me

error log ``` console.trace: chrome://zotero-better-bibtex/content/better-bibtex.js 2907 log chrome://zotero-better-bibtex/content/better-bibtex.js 3187 parseXHTMLToFragment chrome://zotero-better-bibtex/content/better-bibtex.js 3660 node_modules/zotero-plugin-toolkit/dist/managers/preferencePane.js/register/onOpenWindow/< console.groupEnd: zotero(3)(+0000005): {"location":null} console.groupCollapsed: ({}) console.trace: chrome://zotero-better-bibtex/content/better-bibtex.js 2907 log chrome://zotero-better-bibtex/content/better-bibtex.js 3661 node_modules/zotero-plugin-toolkit/dist/managers/preferencePane.js/register/onOpenWindow/< console.groupEnd: zotero(3)(+0000002): {} JavaScript error: chrome://global/content/bindings/tabbox.xml, line 250: TypeError: children[i].getAttribute is not a function JavaScript error: chrome://global/content/bindings/menulist.xml, line 236: TypeError: val.setAttribute is not a function JavaScript error: chrome://global/content/bindings/menulist.xml, line 236: TypeError: val.setAttribute is not a function JavaScript error: chrome://global/content/bindings/menulist.xml, line 236: TypeError: val.setAttribute is not a function JavaScript error: chrome://global/content/bindings/menulist.xml, line 236: TypeError: val.setAttribute is not a function JavaScript error: chrome://global/content/bindings/menulist.xml, line 236: TypeError: val.setAttribute is not a function JavaScript error: chrome://global/content/bindings/menulist.xml, line 236: TypeError: val.setAttribute is not a function JavaScript error: chrome://global/content/bindings/menulist.xml, line 236: TypeError: val.setAttribute is not a function JavaScript error: chrome://global/content/bindings/menulist.xml, line 236: TypeError: val.setAttribute is not a function JavaScript error: chrome://global/content/bindings/menulist.xml, line 236: TypeError: val.setAttribute is not a function JavaScript error: chrome://global/content/bindings/menulist.xml, line 236: TypeError: val.setAttribute is not a function ..... lots more ```
volatile-static commented 1 year ago

There is no need to add <preference> any more. see https://github.com/windingwind/zotero-plugin-template/blob/bootstrap/addon/chrome/content/preferences.xhtml

windingwind commented 1 year ago

A real world example: https://github.com/windingwind/zotero-pdf-translate/blob/main/addon/chrome/content/preferences.xhtml

retorquere commented 1 year ago
windingwind commented 1 year ago

Does this xhtml work on Zotero7? Not sure if this is a bug with toolkit or xhtml itself.

As the does not have any effect, it is natural not to support them. If you have to use a xhtml with , maybe an extra regex in toolkit after loading the xhtml. They’ll be ignored anyway.

retorquere commented 1 year ago

I don't know whether it works on Z7. The source file is only xul.

As the does not have any effect, it is natural not to support them.

I take it you mean "As the preferences does not have any effect"? It was said they would be inert, but as shown in the 2nd gist, if I remove them I get the same problem.

If you have to use a xhtml with ,

With preferences? I don't have to use them, and the 2nd gist doesn't have them.

windingwind commented 1 year ago

I haven’t tried but it seems like the problem is related to tabbox. Maybe they cannot be correctly created with createElementNS? Not sure.

As Z7 suggests developers use groupbox instead of tabbox, I don’t use tabbox anymore. I’ll try your xhtml when I have time.

volatile-static commented 1 year ago

@retorquere Could you show your code where you register this xhtml?

retorquere commented 1 year ago

https://github.com/retorquere/zotero-better-bibtex/blob/zpt/content/prefs.ts#L6-L68 (the errors appear regardless of whether I have the load handler present)

retorquere commented 1 year ago

As Z7 suggests developers use groupbox instead of tabbox, I don’t use tabbox anymore. I’ll try your xhtml when I have time.

That strikes me as strange, because tabbox and groupbox do very different things. How would I get the behavior where you would see a particular pane only when the corresponding tab is selected?

I don't see any mention of tabboxes on zotero-dev or https://www.zotero.org/support/dev/zotero_7_for_developers

retorquere commented 1 year ago

I do see the icon appear in the preferences window BTW, and I see prefs being attached to controls, so the loading appears to at least partially work.

windingwind commented 1 year ago

The Zotero team did not say tabbox is not recommended, while they do not use it anymore. In Z7, the settings that used to be in tabbox are now flattened and in groupbox.

Z7 provides a registration option called subpanel, which creates a hidden panel.

However it is not that easy to implement in Z6. So I would rather not use this subpanel. And actually it is not a tabbox.

I think flatten the settings is natural and reasonable, especially considering that in Z7 the page is scrollable (and in Z6 with toolkit).

retorquere commented 1 year ago

BBT has a lot of prefs grouped into nested tabboxes. Scrolling really isn't an elegant replacement for that. But If I have to, so be it.

windingwind commented 1 year ago

As a developer I know how painful it is to change these prefs to the Zotero 7’s behavior. I spent weeks doing this on my several plugins. I hated the styles and strange xul elements. They breaks the style, cause unexpected line breaks, and sometimes don’t even appear in Z7. (The menulist is invisible in about:blank window). It’s time for XUL elements to retire.

retorquere commented 1 year ago

I personally liked XUL, and I'll miss it.

windingwind commented 1 year ago

Ohh it’s only my personal opinion. The design of XUL elements are very cool. If they are not that buggy I would definitely love them.

windingwind commented 1 year ago

I confirm the <tabbox> can be correctly created with the toolkit.

Zotero 6

image

Zotero 7

image

windingwind commented 1 year ago

And the search of Zotero 7 also works with tabbox and its child elements in Zotero 7.

windingwind commented 1 year ago

I figured out why: the <menulist> item cannot be correctly created with DOMParser in Zotero 6. This is a normal menulist:

image

and this is what DOMParser gives us:

image

Some anonymous elements are not created and this causes the bugs you mentioned above.

My workaround is to put a placeholder element and replace them using ztoolkit.UI.replaceElement.

retorquere commented 1 year ago

Would HTML select/option be a working replacement?

windingwind commented 1 year ago

Would HTML select/option be a working replacement?

Yes, it works on Zotero 6 but fails on Zotero 7 (the popup doesn't show up). I dug deep into the firefox code to see why this happens and it turns out the popup on newer firefox versions requires the window to be somehow initialized while the preference (and most of the other windows) are not.

I made a workaround for the select elements on Zotero 7 here:

https://github.com/windingwind/zotero-plugin-toolkit/blob/d70aa3bca51052f3b1b407b69e76e6fd2c403883/src/helpers/dialog.ts#L386-L461

Anyway, it is very annoying. So my suggestion is to create the menulist in the script instead of XHTML.

retorquere commented 1 year ago

I'll just first see if I can sensibly replace them with radiobuttons instead. Thanks for the deep-dive BTW.

retorquere commented 1 year ago

What namespace should I use for xul elements in the xhtml? I see conflicting information about whether the default (none) namespace is xul or html.

retorquere commented 1 year ago

I can set that with an option I see. I have partial success when I register this prefpane using

manager.register({
      pluginID: 'zotero-better-bibtex',
      src: 'chrome://zotero-better-bibtex/content/preferences.xhtml',
      label: 'Better BibTeX',
      image: 'chrome://zotero-better-bibtex/skin/bibtex.svg',
      extraDTD: ['chrome://zotero-better-bibtex/locale/overlay.dtd'],
      defaultXUL: true,
      onload: async (win: Window) => {
        log.debug('loading prefpane')
        this.window = win
        await this.load()
        log.debug('loading prefpane done')
        win.addEventListener('unload', () => {
          log.debug('unloading prefpane')
          this.unload()
          this.window = null
          log.debug('unloading prefpane done')
        })
      },
    })

but a part of the UI is missing, and the error log has

JavaScript error: chrome://global/content/bindings/tabbox.xml, line 250: TypeError: children[i].getAttribute is not a function
JavaScript error: chrome://global/content/bindings/tabbox.xml, line 250: TypeError: children[i].getAttribute is not a function
JavaScript error: chrome://global/content/bindings/tabbox.xml, line 250: TypeError: children[i].getAttribute is not a function
JavaScript error: chrome://zotero/content/bindings/preferences-mac.xml, line 148: TypeError: this.preferences.rootBranchInternal is undefined
JavaScript error: chrome://zotero/content/bindings/preferences-mac.xml, line 148: TypeError: this.preferences.rootBranchInternal is undefined
[lots and lots of this last line]
windingwind commented 1 year ago

I haven't tried with Mac, not sure if this bug is related to the Mac platform. I'll try to run them on my PC and feedback once I have the results.

The default NS of Zotero seems to be changed at some point in their beta version. Not sure which one will be the default in the final release, so setting it manually is always a good option.

retorquere commented 1 year ago

I'm just going to strip out tabbox. No sense fighting the tide.

retorquere commented 1 year ago

How can I add CSS to the loaded prefpane?

windingwind commented 1 year ago

The CSS can be hard-coded in the xhtml. If you want them to be loaded after the window is ready, please add the loading CSS action, which could be a createElement call, to the onload event callback.

See also:

https://github.com/windingwind/zotero-plugin-template/blob/c4777e35a9aca591d5e52d743cdd376e9a18f5bd/addon/chrome/content/preferences.xhtml#L1-L4

https://github.com/windingwind/zotero-plugin-template/blob/c4777e35a9aca591d5e52d743cdd376e9a18f5bd/src/modules/preferenceScript.ts#L4-L42

retorquere commented 1 year ago

The CSS can be hard-coded in the xhtml

You mean on each individual element? I'd like to avoid that if possible.

which could be a createElement call, to the onload event callback.

So create a <style> element? can those exist interleaved with the HTML body?

retorquere commented 1 year ago

Anyway, it is very annoying. So my suggestion is to create the menulist in the script instead of XHTML.

Will preferences be bound to dynamically created menulists? I will need something like menulist, I tried replacing them all with radiobuttons and that just makes the UI way too big.

What a pain this is. I'm glad your toolkit exists, because I might have given up at this point without it. I hate UI work.

retorquere commented 1 year ago

I tried injecting a processing instruction to load a css file but that doesn't seem to do anything.

windingwind commented 1 year ago

I'll test the