Project

General

Profile

Bug(バグ) #2362

IE9 で文字装飾のプレビューモードが正常に動作しない( tinymce のバージョンを上げる)

Added by Kiwa Sakai about 8 years ago. Updated almost 4 years ago.

Status:
Fixed(完了)
Priority:
High(高め)
Assignee:
Target version:
Start date:
2011-08-22
Due date:
% Done:

100%

3.6 で発生するか:
Yes
3.8 で発生するか:
Unknown (未調査)

Description

概要

TinyMCE (tiny_mce) の文字装飾機能について、IE9 ではプレビューモードが正常に動作しない。なお、この問題は OpenPNE 2 でも発生する。

  • キャプチャ
    文字サイズのサンプル画像
  • 太字・斜体のような文字装飾ボタンを押しても装飾が機能しない
  • 文字色パレット・絵文字パレットは表示されるが、色や絵文字を選んで押下しても装飾が反映されない
  • テキストモードに戻せない
  • 文字の表示サイズが他のブラウザ(IE8 など)に比べて小さい

これらは後述する原因によるものであり、よりマクロな問題は「IE9 で文字装飾機能(TinyMCE)が動作しない」というものである。

IE9のスクリプトデバッグモードを使用した際に表示されるエラー

OpenPNE 3 の場合、テキストモード→プレビューモードの切替時に以下のエラーが表示される。

SCRIPT438: オブジェクトは 'recalc' プロパティまたはメソッドをサポートしていません。 
tiny_mce.js, 行 1 文字19997

このエラーが表示された直後にブラウザがフリーズするため、文字装飾ボタンをクリックした場合などのエラーは確認していない。

(蛇足)OpenPNE 2 の場合は上と同じタイミングで以下のエラーが表示される。

SCRIPT438: オブジェクトは 'parentElement' プロパティまたはメソッドをサポートしていません。 
tiny_mce.js?r7140, 行 1 文字37913

対象バージョン

OpenPNE 3 系の全てのバージョン。

この問題は TinyMCE 側にあり、 IE9 独自の問題に対応したバージョンの TinyMCE を用いていないことが OpenPNE でこの問題が生じる原因である。ただし OpenPNE 側にも問題があるので併せて修正する。

修正方針

2009年10月14日に OpenPNE-3.1.4 で TinyMCE-3.2.7 が組み込まれている( e24edf33 )。 TinyMCE-3.2.7 では IE9 に対応しておらず、上記のような問題が生じる。

2011年8月現在、 TinyMCE は 3.4.4 が最新版として公開されており、このリリースまでに IE9 に対する修正がいくつか取り込まれている。

  • TinyMCE を 3.4.4 にバージョンアップすることで対応する。
  • ただし、 TinyMCE-3.2.7 の実装を前提に記述した OpenPNE 側のコードがあるため、バージョンアップ後に OpenPNE 側で拡張した部分を修正する必要がある。

pc_frontend-diary-edit-decoration_IE.png View - 文字サイズのサンプル画像 (19.5 KB) Kiwa Sakai, 2011-08-22 15:28


Related issues

Related to OpenPNE 3 - Backport(バックポート) #2370: IE9 で文字装飾のプレビューモードが正常に動作しない( tinymce のバージョンを上げる) Fixed(完了) 2011-08-22
Related to OpenPNE 3 - Backport(バックポート) #2799: IE9 で文字装飾のプレビューモードが正常に動作しない( tinymce のバージョンを上げる) Fixed(完了) 2011-08-22
Related to OpenPNE 3 - Task(タスク) #4071: OpenPNEに同梱されている TinyMCE 3.4.4 が IE11 に対応していないため、バージョンアップについて調査する New(新規) 2016-12-20

Associated revisions

Revision 63d8b3f0 (diff)
Added by Minoru Takai almost 8 years ago

(refs #2362) updated tinymce to 3.4.4

Revision 9c785df7 (diff)
Added by Minoru Takai almost 8 years ago

(refs #2362) fixes tinymce-plugins/openpne for tinymce-3.4.4

Revision 15411d5d (diff)
Added by Minoru Takai almost 8 years ago

(refs #2362) changed emoji-palette position.

Revision efb3f429 (diff)
Added by Minoru Takai almost 8 years ago

(refs #2362) fixes buttons spacing of tinymce-toolbar for IE6/7

Revision 8c2c373c (diff)
Added by Minoru Takai almost 8 years ago

(refs #2362) fixed the color-palette of text-mode not to depend on position-property-value of element of ancestors.

Revision b443ff21 (diff)
Added by Minoru Takai almost 8 years ago

(refs #2362) fixes tinymce-plugins/openpne for tinymce-3.4.4; fixes hideMenu of the emoji-palette.

Revision 483690b9 (diff)
Added by Minoru Takai almost 8 years ago

Revert 15411d5d "(refs #2362) changed emoji-palette position."

Revision ed49d5bd (diff)
Added by Minoru Takai almost 8 years ago

(refs #2362) changed to display the emoji-palette on the upper part of the button.

Revision a4d2c23a (diff)
Added by Minoru Takai almost 8 years ago

(refs #2362) added inline-block to the button, that is "a" (anchor) element, to exactly calculate the height.

Revision 2438d77e (diff)
Added by Minoru Takai almost 8 years ago

(refs #2362) fixed _previewToText() method to convert the "strong", "strike" and "em" element in Webkit (Safari, Chrome)

Revision 9b1e8e94 (diff)
Added by Minoru Takai almost 8 years ago

(refs #2362) fixed _previewToText() method not to output many line-break.

History

#1 Updated by Kiwa Sakai about 8 years ago

  • Description updated (diff)

#2 Updated by Minoru Takai about 8 years ago

web/js/tiny_mce/tiny_mce_src.js

1-var tinymce = {
2:    majorVersion : '3',
3:    minorVersion : '2.7',
4-    releaseDate : '2009-09-22',
5-

IE9 で tiny_mce が動作しない問題はいくつも記事がヒットする。 http://stackoverflow.com/questions/6912211/tinymce-not-working-in-ie9

tiny_mce のバージョンを上げてこの問題を解消するのが(バージョンアップによる副作用がなければ)好ましいが、 OpenPNE が IE9 をどのようにサポートするかという別の問題も絡んでくる。

IE8 対応についても同様だが、 <meta http-equiv="X-UA-Compatible" content="IE=EmulateIE7"> を記述することで問題が生じないようにするという方針も考えられる。 IE の過去互換モードを使用して回避するか、それを用いずに IE のバージョン間の際を考慮して対応する(つまり tiny_mce のバグを解消する)かのどちらかが考えられる。

ちなみに、このチケットで扱う内容の対象外かもしれないが、 IE6/7 で文字装飾ボタンの間隔が大きく開いているのは IE6/7 の CSS の解釈に不備があるためであり、これは CSS の追記で修正できる。

考察

現在 TinyMCE のバージョン 3.2.7 を用いているのは、単にそのバージョンに上げた時点で(おそらく)最新版だったためであると考えられる(2009年10月14日に OpenPNE-3.1.4 で TinyMCE-3.2.7 が組み込まれている e24edf33 )。

つまり現在の最新版 3.4.4 にバージョンアップするべきではない理由は特にない。

そして、TinyMCE をバージョンアップせずとも IE 独自の meta 要素によるエミュレーションにより IE9 でも(互換モードで表示させることで)動作するように対応することはできるが、IE9 をネイティブのまま動作しないようにすることは好ましい対応ではない(IE9 の新仕様を使わせないことは、IE のバージョンアップを否定することを意味する)。

IE9 に対して互換モードを用いるようにする、という対応は、それ以外に対応策がなく、互換モードで扱わせてしまうことによるデメリットが許容できるような限定的な場面に限って行うという方針であるべきである。

こうしたことを考慮すると、本問題の本質的な解決策は TinyMCE のバージョンアップであると考えられる。

#3 Updated by Shingo Yamada about 8 years ago

  • Priority changed from Normal(通常) to High(高め)

#4 Updated by Shingo Yamada about 8 years ago

  • 360対象 set to RC1

#5 Updated by Yuma Sakata almost 8 years ago

  • Target version set to OpenPNE 3.7.0

#6 Updated by Shingo Yamada almost 8 years ago

  • Assignee set to Minoru Takai

#7 Updated by Minoru Takai almost 8 years ago

  • Status changed from New(新規) to Accepted(着手)

関連 e24edf33

#8 Updated by Minoru Takai almost 8 years ago

  • Subject changed from IE9 で文字装飾のプレビューモードが正常に動作しない to IE9 で文字装飾のプレビューモードが正常に動作しない( tinymce のバージョンを上げる)
  • Description updated (diff)

#9 Updated by Minoru Takai almost 8 years ago

  • Status changed from Accepted(着手) to Pending Review(レビュー待ち)
  • % Done changed from 0 to 50
  • 3.6 で発生するか set to Yes

対応しました。

修正内容

  1. 63d8b3f0
    • TinyMCE を 3.4.4 にバージョンアップした。
    • 作業手順
      1. http://www.tinymce.com/ から最新版の zip パッケージをダウンロードする。
      2. サーバ上で展開する。 tinymce ディレクトリが作られ、 (OPENPNE_DIR)/web/js/tiny_mce/ と同等のファイルは tinymce/jscripts/tiny_mce/ に含まれている。
      3. tinymce/jscripts/tiny_mce/plugins/ には OpenPNE で用いているもの以外も含まれているため、 plugins/inlinepopups 以外の plugins/* を削除する。
      4. こうして用意した tinymce/jscripts/tiny_mce/ を (OPENPNE_DIR)/web/js/tiny_mce/ に追加する。
        tinymce 側にあるファイルを OpenPNE 側に上書きする。
        tinymce 側にはなく OpenPNE 側にあるファイルは残さなければならない。
        
        これは次のコマンドで実行できる( mv コマンドでは同等のことはできない)。
        
        cp -r tinymce/jscripts/tiny_mce/ (OPENPNE_DIR)/web/js/tiny_mce/
        
      5. これでバージョンアップは完了となる。
  2. 9c785df7
    • tiny_mce/plugins/openpne が TinyMCE-3.2.7 に依存したコードとなっているので修正した。
    • この修正により、カラーパレットと絵文字パレットが今まで通り使えるようになる。
  3. 15411d5d
    • このチケットの問題ではないが、絵文字パレットの表示位置を変更した。
    • これまでの仕様と異なることになるが、本文エリアに被さるように絵文字パレットが表示されるのは利便性が低いと判断した。
  4. efb3f429
    • このチケットの問題ではないが、 IE6/7 で文字装飾ボタンの間隔が広くなっている問題を解消した。
  5. 8c2c373c
    • このチケットの問題ではないが、テキストモードでのカラーパレットの表示位置が、ウィンドウが基点であることを前提としており、カスタマイズなどで「パレットの親要素(厳密には、文字装飾ボタンを囲う div#diary_body_buttonmenu の要素)よりも先祖の要素に position プロパティで static 以外の指定されている場合」に「テキストモードでのカラーパレットの表示位置が想定位置から大幅にずれる」という問題があったのを解消した。
  6. b443ff21
    • 追加修正:絵文字パレット内(絵文字以外の部分)をクリックした後で、絵文字パレットを閉じることができない問題を解消した。

このチケットの問題に対しては、1個目と2個目の修正のみで良いかもしれませんが、3個目から5個目の修正も併せてこのチケットで扱いました。(追記:1個目と2個目と6個目の修正が必要です。)

3個目から5個目の修正は、TinyMCE-3.2.7 時点でもそのまま適用できる修正です。

備考

  • 2個目のコミットで tiny_mce.js.src とか書いているけど、本当は tiny_mce_src.js だった。 tiny_mce/plugins/openpne/editor_plugin.js の方は editor_plugin.js.src となっているのでコメントを記述したときに間違えた。直そうかとも思ったけど意図が分かるからこのままでもいいか。
    // copied from tiny_mce/tiny_mce.js.src (ver 3.4.4), and does not use "t.editor" 
    

実装者として確認した動作テスト項目

  • テキストモードで
    • (1) 普通に文字が入力できる
    • (2) 「太字、下線、打消線、斜体、大きな文字、小さな文字」での装飾が動作する
    • (3) 「カラーパレット」が表示ができる、表示位置が不自然でない、色選択ができる、閉じることができる
    • (4) 「絵文字パレット」が表示ができる、絵文字の表示が不自然でない、絵文字入力ができる、閉じることができる
  • プレビューモードで
    • (5) 文字装飾ボタンが普通に表示される
    • (6) テキストモードに戻れる
    • (7) 普通に文字が入力でき、テキストエリア内での文字列の見栄えが不自然でない(IE9 で文字サイズが小さくなっている問題に対する確認)
    • (8) 「太字、下線、打消線、斜体、大きな文字、小さな文字」での装飾が動作する
    • (9) 「カラーパレット」が表示できる、表示位置が不自然でない
    • (10) 「カラーパレット」を閉じることができる
    • (11) 「カラーパレット」で色を選ぶとパレットが勝手に閉じる(正常動作)
    • (12) 「絵文字パレット」が表示できる、表示位置が不自然でない(これまでと同様であるか、あるいは改善されているか)
    • (13) 「絵文字パレット」を表示した後に、(絵文字を入力せずに)パレット外をクリックすることで、パレットを閉じることができる
    • (14) 「絵文字パレット」を表示し、絵文字をクリックすると絵文字が入力できる(パレットは閉じない)
    • (15) 「絵文字パレット」で (14) の操作に続けて、連続して絵文字を入力できる
    • (16) 「絵文字パレット」で絵文字を入力した後に、パレット外をクリックすることで、パレットを閉じることができる
  • 追加( note-13 以降を参照)
    • (17) 「絵文字パレット」でパレット内の絵文字以外の場所をクリックした後に、パレット外をクリックすることで、パレットを閉じることができる

2個目の修正 9c785df7 では、 (9), (10), (13), (16) を特に修正しています。1個目の修正(バージョンアップ)だけでは、この 4 項目が正常に動作しませんでした。(追記:6個目の修正で (17) を修正しました。)

実装上の補足ですが、このチケットの修正前は、絵文字パレットにおいて「 mousedown で絵文字が入力できていた」のですが、今回の修正で「 click で絵文字が入力できる」ようにしました。 mousedown はマウスで押す操作、 click はマウスで押してその場で離す操作です。ちなみにドラッグはマウスで押してそのまま移動する操作です。つまり、絵文字上でドラッグ操作を行おうとした場合、修正前はマウスで絵文字を押した瞬間に入力されましたが、修正後は「絵文字を押した後にその場で離さない限り入力されない」ように動作が変わっています。 mousedown イベントを用いていると (16) の問題を容易に(hideMenu メソッドをオーバーライドせずに)解消できなかったためです。

と書いていましたが、6個目の修正 b443ff21 で、絵文字の入力を行うイベントを mousedown に戻しました。 (17) の問題を解消するために hideMenu メソッドをオーバーライドする方針で修正したためです。

実装者テストの結果

  • Windows Vista IE9
  • 同 IE-Tester IE8
  • 同 IE-Tester IE7
  • 同 IE-Tester IE6
  • Mac OSX Firefox 6.0
  • 同 Google Chrome 13.0
  • 同 Safari 5.0
  • 同 Opera 11.51

で確認し、上記全てのブラウザで (1) 〜 (16) までの動作が正常に行えることを確認しています。

その後、6個目の修正を行い、 Mac OSX Firefox 6.0 では (1) 〜 (17) までの動作が正常に行えることを確認しています。他のブラウザでも (17) の動作は正常に行えるはずです。

補足: 7個目、8個目、9個目の修正を追加していますが、これは上記の実装者テストについて、どうなっていれば問題ないかという評価基準をより厳しくした上での追加修正です。

#10 Updated by Kousuke Ebihara almost 8 years ago

メモ: tiny_mce/tiny_mce_src.js の renderMenu と tiny_mce/plugins/openpne/editor_plugin.js.src の renderMenu との差分 (手作業で抽出したので変なホワイトスペースが入っている可能性はある)

$ diff -b -u /tmp/original /tmp/openpne
--- /tmp/original    2011-09-01 14:57:08.000000000 +0900
+++ /tmp/openpne    2011-09-01 14:59:34.000000000 +0900
@@ -1,3 +1,4 @@
+// copied from tiny_mce/tiny_mce.js.src (ver 3.4.4), and does not use "t.editor" 
 renderMenu : function() {
     var t = this, m, i = 0, s = t.settings, n, tb, tr, w, context;

@@ -25,11 +26,11 @@
             style : {
                 backgroundColor : '#' + c
             },
-            'title': t.editor.getLang('colors.' + c, c),
+            // 'title': t.editor.getLang('colors.' + c, c),
             'data-mce-color' : '#' + c
         });

-        if (t.editor.forcedHighContrastMode) {
+        if (t.editor && t.editor.forcedHighContrastMode) {
             n = DOM.add(n, 'canvas', { width: 16, height: 16, 'aria-hidden': 'true' });
             if (n.getContext && (context = n.getContext("2d"))) {
                 context.fillStyle = '#' + c;
@@ -54,6 +55,7 @@

     DOM.addClass(m, 'mceColorSplitMenu');

+
     new tinymce.ui.KeyboardNavigation({
         root: t.id + '_menu',
         items: DOM.select('a', t.id + '_menu'),

#11 Updated by Kousuke Ebihara almost 8 years ago

  • Status changed from Pending Review(レビュー待ち) to Pending Testing(テスト待ち)
  • % Done changed from 50 to 70

#12 Updated by Minoru Takai almost 8 years ago

  1. 63d8b3f0
  2. 9c785df7
  3. 15411d5d
    • このチケットの問題ではないが、絵文字パレットの表示位置を変更した。
    • これまでの仕様と異なることになるが、本文エリアに被さるように絵文字パレットが表示されるのは利便性が低いと判断した。
  4. efb3f429
    • このチケットの問題ではないが、 IE6/7 で文字装飾ボタンの間隔が広くなっている問題を解消した。
  5. 8c2c373c
    • このチケットの問題ではないが、テキストモードでのカラーパレットの表示位置が、ウィンドウが基点であることを前提としており、カスタマイズなどで「パレットの親要素(厳密には、文字装飾ボタンを囲う div#diary_body_buttonmenu の要素)よりも先祖の要素に position プロパティで static 以外の指定されている場合」に「テキストモードでのカラーパレットの表示位置が想定位置から大幅にずれる」という問題があったのを解消した。

このチケットの問題に対しては、1個目と2個目の修正のみで良いかもしれませんが、3個目から5個目の修正も併せてこのチケットで扱いました。

5個目の修正を note-11 のレビュー後に追加したのでコメントしておきます(5個目の修正内容自体を追加することに問題はないと思います)。

前述の通り、このチケットの内容だけ考えれば1個目と2個目の修正のみで良いのですが、改善も併せて3個目と4個目を行いました。しかし5個目の問題も認識しており、これも併せて行ってしまっても良いのではないかと思ったので追加しました。

もし3個目以降の修正を取り込んでしまうことが問題であると判断し得る場合は、直近のリリースまでに指摘をお願いします。指摘内容に則して対応をしたいと思います。ただし、チケットが完了となりリリースが行なわれてしまった後で、ここでの修正が不適切であると判断された場合は別チケットで改めて扱うことを予定します。

note-10 に対して

tiny_mce/tiny_mce_src.js の renderMenu と tiny_mce/plugins/openpne/editor_plugin.js.src の renderMenu との差分

についてですが、プレビューモードでのカラーパレット (var t = this) において、 t.editor が undefined であるため t.editor.* を直接参照している箇所を修正しました(※)。

※しかしながら、 tiny_mce/tiny_mce_src.js 側では renderMenu において t.editor が何らかのオブジェクトであることを前提としていることを考えると、 tiny_mce/plugins/openpne/editor_plugin.js.src 側で t.editor を用意できていないことが本質的な誤りなのではないかとも思っている。が、どこを修正すべきか分からなかったので t.editor がオブジェクトとして扱えないことを前提とするコードへと書き換えた(以下参照)。

  • // 'title': t.editor.getLang('colors.' + c, c),
    • 「 t.editor があれば」という記述への変更が難しかったのでコメントアウトしました。
  • if (t.editor && t.editor.forcedHighContrastMode) {
    • 「 t.editor があれば」という記述を追加しました。(「 t.editor && 」を追加)

この 2 箇所を変更しました。 note-10 で示されている下方の差分:

@@ -54,6 +55,7 @@

     DOM.addClass(m, 'mceColorSplitMenu');

+
     new tinymce.ui.KeyboardNavigation({
         root: t.id + '_menu',
         items: DOM.select('a', t.id + '_menu'),

は入っていないと思います(が、もしかしたら誤操作で不要な空白類が混在しているかもしれません)。

#13 Updated by Minoru Takai almost 8 years ago

  • Status changed from Pending Testing(テスト待ち) to Rejected(差し戻し)
  • % Done changed from 70 to 50

実装者テストを改めて行いましたが、 note-9 での動作テスト項目が不十分でした。

(12) 「絵文字パレット」が表示できる、表示位置が不自然でない(これまでと同様であるか、あるいは改善されているか)
(13) 「絵文字パレット」を表示した後に、(絵文字を入力せずに)パレット外をクリックすることで、パレットを閉じることができる
(14) 「絵文字パレット」を表示し、絵文字をクリックすると絵文字が入力できる(パレットは閉じない)
(15) 「絵文字パレット」で (14) の操作に続けて、連続して絵文字を入力できる
(16) 「絵文字パレット」で絵文字を入力した後に、パレット外をクリックすることで、パレットを閉じることができる
(17) 「絵文字パレット」でパレット内の絵文字以外の場所をクリックした後に、パレット外をクリックすることで、パレットを閉じることができる

この新たな項目 (17) が正常に動作していないので修正します。

note-11 でレビューを頂いていますが、これは実装者かテスターでないと気づかないものなので、レビュアーのチェック漏れではないことを示しておきます(つまりこれを独立に修正した後では、実装者の裁量で再レビューを待たずしてテスト待ちにしてよいと判断します)。

#14 Updated by Minoru Takai almost 8 years ago

  • Status changed from Rejected(差し戻し) to Accepted(着手)

#15 Updated by Minoru Takai almost 8 years ago

  • Status changed from Accepted(着手) to Pending Review(レビュー待ち)

b443ff21 で、 note-13 の問題を修正しました。

「絵文字パレットが閉じないことがある」問題を解消するために、絵文字パレットに対する hideMenu メソッドを上書きし、 hideMenu が呼ばれたときは t.isMenuVisible の値にかかわらず hideMenu の処理を実行するようにしました。

note-12 の後半の内容(t.editor を使わないようにする)についても、実行する想定でないコードは全てコメントアウトするように変更しました。

また、変更箇所が分かるよう、コードの一部を変更した部分に "// OpenPNE:" という接頭辞をつけてコメントを記述しました。

#16 Updated by Minoru Takai almost 8 years ago

  • Status changed from Pending Review(レビュー待ち) to Pending Testing(テスト待ち)
  • % Done changed from 50 to 70

#17 Updated by Minoru Takai almost 8 years ago

  • Status changed from Pending Testing(テスト待ち) to Rejected(差し戻し)
  • % Done changed from 70 to 50

繰り返しすみません。プレビューモード時の絵文字パレットの位置を変更した3個目の修正が不適切だったので修正し直します。

3個目の修正では、絵文字パレットが「ウィンドウに対しての横軸中央」に表示されますが、絵文字パレットボタンの位置や、テキストエリアの位置が「ウィンドウの横軸中央あたり」にある前提でなければ適切な表示位置には成り得ません。例えば #ContentsContainer { margin: 0 } をカスタムCSSで追加した場合、OpenPNEのPC版のページでは全体が左寄せになりますが、3個目の修正によって絵文字パレットが中央に表示されてしまいます。

#18 Updated by Minoru Takai almost 8 years ago

  • Status changed from Rejected(差し戻し) to Accepted(着手)

#19 Updated by Minoru Takai almost 8 years ago

  • Status changed from Accepted(着手) to Pending Review(レビュー待ち)

改めて修正内容をまとめます。

  1. 63d8b3f0
    • TinyMCE を 3.4.4 にバージョンアップした。
  2. 9c785df7
    • tiny_mce/plugins/openpne が TinyMCE-3.2.7 に依存したコードとなっているので修正した。
    • この修正により、カラーパレットと絵文字パレットが今まで通り使えるようになる。(追記:ある手順を行うと、絵文字パレットを閉じることができない問題が残っている)
  3. 15411d5d
    • このチケットの問題ではないが、絵文字パレットの表示位置を変更した。(追記:文字装飾ボタンの位置を考慮していないため、想定外の位置に表示される問題がある)
    • これまでの仕様と異なることになるが、本文エリアに被さるように絵文字パレットが表示されるのは利便性が低いと判断した。
  4. efb3f429
    • このチケットの問題ではないが、 IE6/7 で文字装飾ボタンの間隔が広くなっている問題を解消した。
  5. 8c2c373c
    • このチケットの問題ではないが、テキストモードでのカラーパレットの表示位置が、ウィンドウが基点であることを前提としており、カスタマイズなどで「パレットの親要素(厳密には、文字装飾ボタンを囲う div#diary_body_buttonmenu の要素)よりも先祖の要素に position プロパティで static 以外の指定されている場合」に「テキストモードでのカラーパレットの表示位置が想定位置から大幅にずれる」という問題があったのを解消した。
  6. b443ff21
    • 追加修正:2個目の修正では不足していた修正を行った。
    • 絵文字パレット内(絵文字以外の部分)をクリックした後で、絵文字パレットを閉じることができない問題を解消した。
  7. 483690b9
    • 3個目の変更は不適切だったため取り消した。
  8. ed49d5bd
    • 3個目の修正に代わる、文字装飾ボタンの位置を考慮した絵文字パレットの表示位置変更を行った。
  9. a4d2c23a
    • この修正は、実装者テスト項目「(3) テキストモード時のカラーパレットの表示位置」について、より適切な位置に表示されるように改善したものです。
    • このチケットの問題ではないが、もともと Safari, Chrome ではテキストモードでのカラーパレットが、上方向に若干ずれていた問題を修正した。これは TinyMCE-3.2.7 以前の問題である(TinyMCE-3.2.7 を用いている場合はプレビューモードでも生じる)が tiny_mce/plugins/openpne では以前のコードを流用しているため TinyMCE-3.4.4 に上げた今でもずれが生じていた。
    • また、5個目の修正を行ってしまうと IE6/7 の不具合によりテキストモードでのカラーパレットが下方向に大幅にずれてしまっていたが、この9個目の修正で解消できる。

  • 7個目と8個目の修正を追加しました。
    • (18) カスタムCSSで #ContentsContainer { margin: 0 } を追加した上で、「プレビューモードの絵文字パレット」の表示位置が適切である
    • (19) カスタムCSSで #Header { display: none } #ContentsContainer { margin-left: -300px } を追加した上で、「プレビューモードの絵文字パレット」がウィンドウ外に出ずに(ウィンドウ左上をパレットの左上として)表示される

(18), (19) は CSS が変更された際の動作であるため、動作テスト項目としては必須ではないかもしれない。

#20 Updated by Minoru Takai almost 8 years ago

  • Status changed from Pending Review(レビュー待ち) to Pending Testing(テスト待ち)
  • % Done changed from 50 to 70

note-11 のレビューを以て、修正方針は適切であると判断し、それ以降に行った修正は私がレビューしたものとします(追記:9個目の修正を追加しましたが、これも含めてレビュー済みとします)。

テストされる方は note-9 での動作テスト項目 が参考になるかもしれません。

#21 Updated by Minoru Takai almost 8 years ago

  • Status changed from Pending Testing(テスト待ち) to Rejected(差し戻し)
  • % Done changed from 70 to 50

http://redmine.openpne.jp/issues/2370#note-6 の問題があるため差し戻します。

#22 Updated by Minoru Takai almost 8 years ago

  • Status changed from Rejected(差し戻し) to Accepted(着手)

note-21 の問題(「太字」の装飾を行うと、プレビューモードからテキストモードに戻すときに Safari, Chrome で <op:b> ではなく <strong> が出力され、「斜体」では <op:i> ではなく <em> が出力される)について対応します。

#23 Updated by Minoru Takai almost 8 years ago

問題の背景

note-21 で指摘された問題は、TinyMCEで入力された要素(スタイル)をOpenPNE独自にパースする処理について、TinyMCEのバージョンと入力者が使うブラウザによって、出力される要素が異なること(例えば「太字」の装飾に b 要素が使われたり strong 要素が使われたりする)が根本的な原因でした。

これには、OpenPNE独自のJS(plugins/openpne/editor_plugin.js)側で、あるブラウザのときはこの要素を独自タグに変換する、といったことを列挙して記述していたようです。しかし今回のTinyMCEのバージョンアップで、この列挙にあてはまらないパターン(例えば Safari ブラウザで strong 要素が使われるケース)が発生したために、独自タグに変換されない要素が生じてしまったようです。

生じている問題

このチケットの対応後(TinyMCE-3.4.4 へのバージョンアップ後)に、 Safari, Chrome にて、プレビューモードで「太字」「斜体」の装飾を行いテキストモードに戻すと、 op:b, op:i ではなく strong, em タグでマークアップされてしまっている(※)。

以前のコメントに示した実装者テスト項目で示すと、以下の動作が Safari, Chrome にて正常ではなかったことになる。

  • (20) プレビューモードで文字装飾を行い、テキストモードに変えたときに、それぞれの装飾に対応した独自タグ(<op:*>)が出力される

※これにより何が問題かという話だが、テスターから報告された問題点は「続けてプレビューモードにしたときに装飾が反映されない」というものだった。例えば太字に関して、 strong タグが出力されていても装飾が反映されればいいかという点だが、これはそもそも意図していないはずであり、独自タグを用意している意図がなくなってしまう( <strong> を独自タグだと言い張るのは可能かもしれないが、それは想定していないはずである)。

余談だが、逆にバージョンアップ前は打消線で装飾すると Safari, Chrome で strike タグになってしまう問題があるようだ(今回の検証中に発見した)。

対応方針の検討

対応方針を考えるため、 web/js/tiny_mce/plugins/openpne/editor_plugin.js.src で、「プレビューモードからテキストモードにモードを変更した瞬間に、プレビューモードで入力されていた要素(スタイル)を独自タグに変換する処理」の変更履歴を追ってみた。

  1. master ブランチで editor_plugin.js.src を git blame して行の差分履歴の大半を占めたコミットログ
    commit 9ec05e7c567acaed0e74356b8a228b45d8d08208
    Author: ebihara <ebihara@8bbc783d-d51a-0410-adab-8fa69d43f207>
    Date:   Wed Dec 17 16:48:49 2008 +0000
    
        #3194:added ability to set widets to home
    
        git-svn-id: https://trac.openpne.jp/svn/OpenPNE3/trunk@9582 8bbc783d-d51a-0410-adab-8fa69d43f207
    
  2. trac.openpne.jp のチケット 3194 http://trac.openpne.jp/ticket/3194 から追ったリビジョン 9582
  3. ここから辿れる web/js/tiny_mce/plugins/openpne/editor_plugins.js.src の最終リビジョン
  4. 9151: コピー元のブランチのリビジョンログを辿る
  5. 7138: コピー元のブランチのリビジョンログを辿る
  6. 7131: コピー元のブランチのリビジョンログを辿る
  7. 6703: コピー元のブランチのリビジョンログを辿る
  8. 6347: コピー元のブランチのリビジョンログを辿る
  9. strong, strike, em 要素をそれぞれ b, s, i 要素に変換する処理を追加した変更
web/js/tiny_mce/plugins/openpne/editor_plugin.js.src
532 行目

if (!tinymce.isWebKit) {  // not safari
    s = tinymce.trim(s);
    rep('/<(\/?)strong>/gi', '<\1b>');
    rep('/<(\/?)strike>/gi', '<\1s>');
    rep('/<(\/?)em>/gi', '<\1i>');
    editor.dom.setHTML(editor.getBody(), s);
}

この部分の if 文を外す( WebKit, つまり Safari や Chrome でもこの処理を行う)ようにすれば、今回の問題は解消されるようだ。

修正内容の確認

このコメントを書いている時点で考えなければならないのは、 http://trac.openpne.jp/changeset/6254 の修正を行ったときと同じ視点で、現状のブラウザの状況に合わせて同様の修正を追加することです。

  • WebKit に対しても strong, strike, em 要素を変換する処理を行うようにするだけで良いのか
    • むしろもともと WebKit を対象外にしていた理由はなんだろうか(WebKit でもこの処理を通してしまってもよいが通す必要がないから対象外にしていたのか、WebKit ではこの処理を通してしまうと問題があったのか)
  • WebKit (for Safari) に向けた処理を削除するなどしなくてもよいか
    • なんでそもそも for safari とかいったコードを書く必要があったのだろうか
  • 他のブラウザで同様の問題が起こっていないか

これらが、特に確認しなければならない点です。現状の web/js/tiny_mce/plugins/openpne/editor_plugin.js.src のソース(本チケットの8個目の修正時点)は次のURLから確認できます。

https://github.com/openpne/OpenPNE3/blob/ed49d5bd970f2e640f1bc9bd6f852b9eceaaf5bb/web/js/tiny_mce/plugins/openpne/editor_plugin.js.src#L525

また、この修正に併せて、 convertHtmlTagToDecoTag() メソッドの一般化(より簡潔な記述)などが可能かどうかも検討しておきたいです。

余談ですが、 590行目 の "// for safari" というコメントは、恐らく fontSize の分岐が加わった時点で不適切なコメントとなっています。 if (org_tagname == 'span') を通り必要な処理が施されるのは Safari (WebKit) だけではありません。コメントの記述も可能なら改善しておきたいです。

#24 Updated by Minoru Takai almost 8 years ago

  • Status changed from Accepted(着手) to Pending Review(レビュー待ち)

2438d77e で、Safari や Chrome でも、プレビューモードで太字・斜体の装飾を行い、テキストモードに変更したときに独自タグに変換されるように修正しました。

太字、打消線、斜体 の装飾としてプレビューモードで strong, strike, em 要素が使われた場合、

if (!tinymce.isWebKit) {  // not safari
    s = tinymce.trim(s);
    rep('/<(\/?)strong>/gi', '<\1b>');
    rep('/<(\/?)strike>/gi', '<\1s>');
    rep('/<(\/?)em>/gi', '<\1i>');
    editor.dom.setHTML(editor.getBody(), s);
}

の処理によって WebKit 以外ではそれぞれ b, s, i 要素に変換されていましたが、WebKit ではこれを行なっていませんでした。それは、この部分の実装当時、WebKit では strong, strike, em 要素が使われることがなく、全て span 要素で表現されていたためです。また、その当時に不要な(しかし通ってもよい)処理を行わない目的で、上記の if 文が記述されていたと判断したため、今回の修正でこの if 文を排除し、全てのブラウザで上記の処理が行なわれるようにしました。

修正箇所付近のコードを見る限り、他のブラウザで同様の問題が起こっている可能性は考えなくてよいと判断しています(そのような可能性が極めて低いと判断している)。また、この修正による副作用はないと判断しています。

#25 Updated by Minoru Takai almost 8 years ago

  • Status changed from Pending Review(レビュー待ち) to Pending Testing(テスト待ち)
  • % Done changed from 50 to 70

レビューを通します。

#26 Updated by Minoru Takai almost 8 years ago

note-23 で editor_plugins.js.src の修正の経緯を示していますが、 http://trac.openpne.jp/changeset/5911 にあるようなコミットで対応した問題が、今回のバージョンアップによる仕様変更で再び問題となっているようです。

TinyMCE のコードをきちんと追っていないので、どこを修正することが根本的な対応なのか判断できていませんが、TinyMCE-3.2.7 の仕様(おそらく TinyMCE のバージョンが影響している)に基づいて記述していたコードが、TinyMCE-3.4.4 へのバージョンアップによって、想定通りの動作となっていない箇所がいくつかあります。 note-21 での指摘もその一つですが、 editor_plugins.js.src がどういう状況を前提に処理を行なっているのかを調べないと、こうした若干の不具合を含んだままのリリースとなってしまう危険性があります。

note-23 でも示していますが、本件は TinyMCE がどのような仕様なのかに加え、各種ブラウザがどのように動作するのかまでを把握していないと、あるケースにおける問題を網羅的に知ることができません(テスト項目が作れません)。

直近では、プレビューモードからテキストモードに変えた際に、改行が倍になってしまう問題が見つかっており、 http://trac.openpne.jp/changeset/5911 を取り消すような形で修正を行えば解消されるようですが、 br 要素を \n 2個分に変換すべきなのか、1個分に変換すべきなのか直ぐに判断ができません(なぜ古いコードでは2個にしていたのか)。

editor_plugins.js.src の記述内容が妥当かどうかについて見直したほうが良いと思いますが、この件については OpenPNE-3.6.0 リリース後の対応となっても良いと思います。つまり別チケットで扱うことになるかもしれません。

#27 Updated by Minoru Takai almost 8 years ago

  • Status changed from Pending Testing(テスト待ち) to Rejected(差し戻し)
  • % Done changed from 70 to 50

現時点で見つかっている、「改行が倍になる」問題をこのチケットで併せて修正することにします。

RC1 リリース後は、本件に関わる細かなバグは別チケット対応とし、 3.6.0 リリース後にマイナーバージョンで対応することとします。

#28 Updated by Minoru Takai almost 8 years ago

  • Status changed from Rejected(差し戻し) to Accepted(着手)

#29 Updated by Minoru Takai almost 8 years ago

  • Status changed from Accepted(着手) to Pending Review(レビュー待ち)

11個目のコミット 9b1e8e94 で note-26 の問題を修正しました。

この修正については、現状のブラウザでの動作を確認し、修正前に対し、修正後の方が動作が適切になることを確認して修正内容を決定しました。

IE6, IE9, Firefox 6.0, Safari 5.0, Chrome 14, Opera 11.51 にて、修正前はこれら全てのブラウザで、1個だった改行が2個になってしまっていましたが、修正によって改行が1個だけ出力されるようになることを確認しています。

#30 Updated by Minoru Takai almost 8 years ago

  • Status changed from Pending Review(レビュー待ち) to Pending Testing(テスト待ち)
  • % Done changed from 50 to 70

レビューを通します。

#31 Updated by Minoru Takai almost 8 years ago

note-30 まででは、細かい不具合が見つかった場合に都度修正を行なっていましたが、OpenPNE-3.6RC1 のリリースに併せて、これ以上の不具合については別チケット扱いとします。

ここまでの修正内容を本チケットで扱うこととします。OpenPNE-3.6RC1 リリース後に、もし本チケットに対して差し戻しがあった場合も、既にバックポートチケットを完了にしている関係で別チケットで対応することにします。

#32 Updated by Yuma Sakata almost 8 years ago

  • Status changed from Pending Testing(テスト待ち) to Fixed(完了)
  • % Done changed from 70 to 100

テストOKです。

#33 Updated by kaoru n almost 4 years ago

  • 3.8 で発生するか set to Unknown (未調査)

#34 Updated by kaoru n over 2 years ago

  • Related to Task(タスク) #4071: OpenPNEに同梱されている TinyMCE 3.4.4 が IE11 に対応していないため、バージョンアップについて調査する added

Also available in: Atom PDF