946647 - Remove bidi attributes in nsIMarkupDocumentViewer as dead code

Status: RESOLVED FIXED

(Core :: Internationalization, enhancement)

Description

[Henri Sivonen (:hsivonen) (away from Bugzilla until 2026-08-27)]

nsIMarkupDocumentViewer::bidiTextDirection looks like dead code:
https://mxr.mozilla.org/comm-central/search?string=bidiTextDirection

Comment 1

[Henri Sivonen (:hsivonen) (away from Bugzilla until 2026-08-27)]

Except it's used by this extension: https://addons.mozilla.org/en-US/firefox/addon/toolbar-buttons/ . Sigh. I wonder if that button in the button pack actually has users.

Comment 2

[Henri Sivonen (:hsivonen) (away from Bugzilla until 2026-08-27)]

Attachment: Zap the code

Anyway, here's a patch.

Comment 3

[Henri Sivonen (:hsivonen) (away from Bugzilla until 2026-08-27)]

Comment on attachment 8343007 [details] [diff] [review]
Zap the code

Requesting review anyway, since the add-on probably shouldn't be offering a directionality change button for all pages. The only use case is flipping between ISO-8859-8-I and ISO-8859-8, which the add-on could do via nsIDocShell::charset.

Comment 5

[Simon Montagu :smontagu]

> Requesting review anyway, since the add-on probably shouldn't be offering a
> directionality change button for all pages. The only use case is flipping
> between ISO-8859-8-I and ISO-8859-8, which the add-on could do via
> nsIDocShell::charset.

FWIW, that would be the use case of a button which toggled bidiTextType, not bidiTextDirection. I haven't tested, but I imagine that the add-on has already been broken by bug 151407. To fix it, it probably needs to do something similar to SwitchDocumentDirection in browser.js.

Comment 6

[Simon Montagu :smontagu]

Comment on attachment 8343007 [details] [diff] [review]
Zap the code

Review of attachment 8343007 [details] [diff] [review]:
-----------------------------------------------------------------

In fact this is orthogonal to bug 867936, even though the patches overlap. Removing it doesn't affect the functionality of the prefs as far as I can tell. Thanks!

Comment 7

[Henri Sivonen (:hsivonen) (away from Bugzilla until 2026-08-27)]

Thanks. Landed:
https://hg.mozilla.org/integration/mozilla-inbound/rev/563cf3e6f372

Comment 8

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/563cf3e6f372