eyalroz / bidimailui Goto Github PK

In BiDiMailUI.Display.wrapTextNodesInFlowedMessageDOMTree(), we try to wrap some elements in a DIV. But - we can't create that as a XUL element.

The preferences dialog doesn't load in TB 68.

The error message complains about a missing namespace for the html:input tag. And, indeed, I somehow forgot to add the sub-namespace for html tags. Let's do that.

Our code currently sets the message subject field's direction like so:

document.getElementById("msgSubject").inputField.style.direction = direction;

the id is correct, but the element is now an html:input, not a XUL <textbox>. See description of this change. We should use a version check for choosing how to set the direction.

We've already collected most of the preferences utility code in BiDiMailUI.Prefs, but in bidimailui-display-logic.js there's a snippet which gets a TB pref semi-directly. Let's add Prefs.getStringPref() and Prefs.getAppStringPref() methods.

In TB 78, getAnonymousElementByAttribute() is no longer supported, and we throw an exception trying to obtain the displayed message's subject. We may also be failing to obtain it in TB 68; and - we may be failing to set it, once the charset changes, using our setter function.

Let's try to rewrite that piece of code in the messenger onLoad() handler to properly support what works in TB 68 and 78.

createElement() can no longer be used to create XUL elements - which are what we typically create - with Thunderbird 69 and above. Instead, we'll use createXULElement(), which is available in Thunderbird 68 as well.

Hi @eyalroz,

Thank you for your efforts maintaining BiDi add-on. I'm trying to change default text encoding through add-on preferences but I'm getting below error message:

XML Parsing Error: prefix not bound to a namespace
Location: chrome://bidimailpack/content/bidimailpack-prefs-68.xul
Line Number 59, Column 7: <html:input id="space_between_paragraphs_value_text"
------^

BiDimailui version: 0.10.1b2
TB version: 68.10.0 under Ubuntu 18.04

Thunderbird has provided - for quite some time now - services with easy lazy getters of references to several popular services - in Services.jsm, (all-Mozilla-relevant) and MailServices.jsm (Mail-andnews-relevant). Let's use those whenever relevant instead of defining our own getters and referencers.

At some point (possibly before the TB 78 conversion), the default message direction and/or reply direction preference stopped working - at least for text messages - and now all text replies are set to LTR.

In some cases, multiple onLoad() events fire for the same message, even without our charset forcing.

It would be nice if we tried to avoid repeating some of our work on each of those calls.

Our message (display) pane context menu entry cycles the direction setting mode: Autodetect, RTL, LTR and back again. But our label says "Switch Message Direction". Let's change that.

Also, the relevant key's ID mentions "switch" rather than "cycle".

In some cases - specifically, when reading RSS-obtained documents sometimes, I can't cycle directions:

  cycleDirectionSettings : function() {
    var messagePane = document.getElementById("messagepane");
    var body = messagePane.contentDocument.body;
    var newForcedDirection;
    switch (body.getAttribute('bidimailui-forced-direction')) {
      case 'ltr':
        newForcedDirection = 'rtl';
        break;
      case 'rtl':
        newForcedDirection = null;
        break;
      default: // should be null
        newForcedDirection = 'ltr';
    }

messagepane is not null, but the contentDocument is missing/null. So, we fail to get the body.

We should explicitly refer to our icon in manifest.json.

Instead of surrounding every debug-logging with preprocessor directives, we could centralize them in one place - a wrapper for console.log() within the RemoveDupes object - and have that wrapper be aware of which DEBUG_XYZ values were set on compilation. Specifically, those could be strings in a set / entries in a dictionary, which would be looked up before actually calling console log; and the wrapper would be invoked with the relevant string.

Ctrl+LShift and Ctrl+RShift should work not just in the message body, but also in the subject box; right now, they don't.

editor.setParagraphFormat() has apparently not worked for quite some time (perhaps since 2016)? We need to use something else:

editor.document.execCommand("defaultparagraphseparator", true, "p");

It's been over 10 years since the extension was called bidimailpack - it's time to finally retire this name!

Having the version number checked in with the dobuild script means a lot of "noise" in the commit history for version number changes. Let's just drop that and require specifying the number on the command-line.

At some point - before TB v68 - the message pane has changed. It used to be popup id="messagePaneContext", now it's menupopup id="mailContext". We need to update our code accordingly.

Recently released Thunderbird 78.0 has a new compose interface. I believe this is what broke the extension.
There are no RTL buttons when you compose a message.
See attached screenshot.

Machine details:
Windows 10 Pro (Version 2004 19041.388)
Thunderbird 78.0
BiDi UI version: 0.10.1b2

It used to be somewhat of a big deal to get access to the console - but it hasn't been already for quite a while. So instead of calling RemoveDupes.JSConsole.logStringMessage() (and defining it) - let's just use console.log().

In all of our chrome folders, we have an extra bidimailpack/ subdirectory which isn't really necessary. (Perhaps it was necessary with SM 1.x?) - let's move everything one subdir upwards.

We get this warning:

When rendering the element, the used values of CSS properties “writing-mode”, “direction”, and “text-orientation” on the element are taken from the computed values of the element, not from the element’s own values. Consider setting these properties on the :root CSS pseudo-class. For more information see “The Principal Writing Mode” in https://www.w3.org/TR/css-writing-modes-3/#principal-flow INBOX>180552

Perhaps it's because we set the HTML element's class to "bidiui-use-detected-directions"? We could, instead, set the body and the head's class to that, instead.

We have some pieces of chrome which regards the preferences dialog which are named with a "prefs" suffix. Let's clarify by a rename.

Mozdev.org will soon be dismantled/taken offline. It's time we switched the website link for the extension to this "page"/mini-site.

In Thunderbird 78, extensions don't know about skins (well, not the way they used to); and neither does John Bieling's privileged extension loader. That means we must "fold" our skin-related files into the content directory. We'll put it in a skin subdirectory.

We used to have:

editor.QueryInterface(nsIEditorStyleSheets);
editorStyle.addOverrideStyleSheet("chrome://bidimailui/content/quotebar.css");

but this no longer works. We need to add the stylesheet some other way.

Installing on recent thunderbird (version 91) produce error.

Could not be installed because it is not compatible with thunderbird 91 

Thunderbird decided to drop the preference mailnews.view_default_charset. So - we can no longer rely on it for the default single-byte charset to assume we've gotten.

While in the longer term we should simply avoid making any such assumption, for now let's switch to a preference of our own.

We used to need code for splitting paragraphs on Enter and re-unifying them on Backspace at paragraph start - to make sure the direction settings were proper.

This is no longer necessary - as the editor does this well enough all on its own, if we set the mail.compose.default_to_paragraph preference to true.

So - let's drop this code!

When no message is focused, you either get an empty message pane or a Thunderbird project message, encouraging donations etc.

We should detect those case and avoid taking any direction and charset related action then.

Perhaps lack of a URI is an appropriate check?

I'm using BiDimailui 0.10.1 with TB 68.10 & BiDimailui 0.11b2 with TB 78.1.1 and I notice strange behavior with text encoding while viewing emails specifically (unicode, Arabic Windows & western text encoding), where RTL & LTR text are not aligned correctly.
Find pic sample.

.
This issue is only with view email, but if I reply, forward, compose email, alignment will be fine!

We want to obtain the "message browser" to use the properties of the displayed copy of a message when composing its reply. Alas, the code win.getMessageBrowser() which once used to work - has not worked, probably for many years already.

With the TB-78-compatible - injected - versions, direction autodetection fails quietly; not sure why.

We should inject/append stylesheets to a message as children of the head element, not of the html element like we do now.

In some (many?) cases, we load/inject our two per-document stylesheets - direction-autodetection.css and quotebar.css - twice.

Either we figure our where this injection is redundant, or it's not actually redundant and just occasionally gets triggered more than once from the same place - in which case we should check the list document.styleSheets before appending.

At the moment, in order to invoke setAppStringPref(), we need to instantiate an nsISupportsString, and set its data member. It shouldn't be like this - we should be able to just pass a plain JS string, or a supports-string, and letr the method sort things out for us.

We can write

doStuff(arg1, {opt1= def1, opt2 = def2}) { 
  // do stuff; 
}

let' use that where appropriate.

The extension currently loads/injects two stylesheets of its own into displayed messages: quotebar.css and direction-autodetection.css. When the extension is unloaded while a message is displayed, and without the app going down too, perhaps should probably unload those stylesheets. Possibly even unset all of the attributes we set before.

We used to DOM document for the displayed message using:

this.docShell.contentViewer.DOMDocument

from a BiDiMailUI.MessageOverlay method. But - this no longer works (!). Instead, we'll go for:

msgWindow.messageWindowDocShell.contentViewer.DOMDocument

which still works. Weird.

In modern Javascript, we can write:

let foo = `Voxel coordinates are (${x}, ${y}, ${z})`;

while in the past we had to write this add

let foo = 'Voxel coordinates are (' + x + ', ' + y + ', ' + z + ')';

the former is easier to type and less easy to get confused about. Let's use it then.

When replying to a message, but for some reason the displayed message's direction is unavailable (which itself may be a bug), we need to check the reply body's direction.

This currently fails, with an attempt to manipulate an object which shouldn't be undefined,, but somehow is.

Uncaught TypeError: obj is undefined
directionCheck chrome://bidimailui/content/bidimailui-common.js:643
setInitialDirection chrome://bidimailui/content/bidimailui-composer.js:721
NotifyComposeBodyReady chrome://bidimailui/content/bidimailui-composer.js:119
workaroundForcingTimeoutId chrome://bidimailui/content/bidimailui-composer.js:784
bidimailui-common.js:643:9

Now that we rely on mail.compose.default_to_paragraph to have per-paragraph direction abilities, we should set that to true when we're installed - say, about when we also check the default chrarset.

We could announce doing this right after the dialog with the cp1255-vs-cp1256.

In bidimailpack-composer.js, we have the following line:

    if (node && (nodeType in node) 

that's wrong. It should be:

    if (node && ('nodeType' in node) 

there may be other such places.

I'm not sure it worked in 91.0, but in 91.1 the toolbar buttons are not there and Ctrl+Shift+R/L is not working either.

We used to create a style element for adding to a message being composed, like so:

  document.getElementById("content-frame").contentDocument.createXULElement('style');

but this no longer works; possibly, the contentDocument is no longer a XUL document and can't create XUL elements. Instead, it has to be an HTML style element. Also, let's make this code look a bit like the stylesheet link injection function.

Sometime between TB 68 and 78, the menubutton XUL element was reworked, to use the is= attribute. For our menubutton to continue working in 78 and later, we must add is="toolbarbutton-menu-button" to its attributes.

At some point - likely between 68 and 78 - the prompt service' select() method stopped taking array lengths separately from arrays. So, we need to drop the extra argument too.

There is no benefit (that I know of) in using .jar files within the extension's XPI - and it hinders the transition to TB 78 support. So, let's drop the use of JARs.

Seems

<ul>

aren't RTL'ed correctly, it produce something like this

  <body bidimailui-charset-is-forced="true" style="direction: ltr;">
    <p style="direction: rtl;">שלום עולם</p>
    <ul>
      <li>שלום</li>
      <li><br>
      </li>
    </ul>
  </body>

also the

<HR>

break things , but I think it was a bug in the previous release also
(because it does not produce P object, it just uses BR for new line,

<!DOCTYPE html><html style="direction: ltr;">
  <head>

    <meta http-equiv="content-type" content="text/html; charset=">
    <style id="bidiui-paragraph-margins" type="text/css">body p { margin-bottom: 0cm; margin-top: 0pt; } </style>
  </head>
  <body bidimailui-charset-is-forced="true" style="direction: ltr;">
    <p style="direction: rtl;">שלום עולם</p>
    <blockquote>
      <p style="direction: rtl;"><b>שדג</b> 🙁</p>
      <hr width="100%" size="2">שלום<br>
      <br>
    </blockquote>
  </body>
</html>

For comparison - See how in english it keeps using P correcttly

<!DOCTYPE html><html>
  <head>

    <meta http-equiv="content-type" content="text/html; charset=">
  </head>
  <body>
    <p>shalom</p>
    <hr width="100%" size="2">
    <p>this line is nexct</p>
    <p>sadf</p>
    <p><br>
    </p>
    <p><br>
    </p>
  </body>
</html>

Without the addon.
Startting with:

<!DOCTYPE html><html>
  <head>
    <meta http-equiv="content-type" content="text/html; charset=">
  </head>
  <body>
    <p>shalom</p>
    <p style="direction: rtl;">שלום</p>
    </p>
  </body>
</html>

Pressing Enter the H.Line Icon actually worked fine and produced working result

<!DOCTYPE html><html>
  <head>
    <meta http-equiv="content-type" content="text/html; charset=">
  </head>
  <body>
    <p>shalom</p>
    <p style="direction: rtl;">שלום</p>
    <hr width="100%" size="2">
    <p style="direction: rtl;"><br>
    </p>
  </body>
</html>

Tables actually works with no problem.
Beside that it is OK.