Bug(バグ) #2000
プレビューモードの絵文字パレットの右上にスペースがある
100%
Description
現象¶
プレビューモード時の絵文字パレットの右上(「テニスの絵文字」の右)にスペースがある(添付画像参照)。テキストモード時には絵文字パレットはインラインで表示されるためこの問題はない。
原因¶
- web/js/tiny_mce/plugins/openpne/editor_plugin.js.src
web/js/tiny_mce/plugins/openpne/editor_plugin.js.src 55- for (var num in s.emoji) { 56- var emoji = s.emoji[num]; 57- for (var i = emoji.start; i <= emoji.end; i++) { 58: if (i == emoji.start || i % 25 == 0) { 59- tr = DOM.add(tb, 'tr'); 60- }
この 58 行目では (i == emoji.start || i % 25 == 0) としているが、(i <= emoji.end と等号を含んでいることからも分かるように) i は [0,len) ではなく [1,len] の値を取ることが想定されている。
58 行目が (i == emoji.start || i % 25 == 0) である場合、
- i=1 のとき、 emoji.start なので新たな行を開始( [1, 24] がこの行に含まれる)
- i=25 のとき、 i%25 == 0 なので新たな行を開始( [25, 49] がこの行に含まれる)
- i=50 のとき、 i%25 == 0 なので新たな行を開始( [50, 74] がこの行に含まれる)
- i=(25*n) のとき、...
[1, 24] では 1 から 24 までの 24 個しか含まれない。このせいで 1 行目には 25 個目の絵文字が表示されていない。
修正内容¶
web/js/tiny_mce/plugins/openpne/editor_plugin.js.src 58行目の if 文
if (i == emoji.start || i % 25 == 0) {
を次のように書き換える。
if ((i - 1) % 25 == 0) {
i が emoji.start と一致しているかを評価する必要はない。
- 追記
- for 文では i = emoji.start とループ変数を初期化しており、 i が 1 から始まることは必要条件としていない。
for (var i = emoji.start; i <= emoji.end; i++) { if ((i - 1) % 25 == 0) { tr = DOM.add(tb, 'tr'); }
- このため、 if 文では (i - 1) ではなく (i - emoji.start) と書き換えることにする。
- for 文では i = emoji.start とループ変数を初期化しており、 i が 1 から始まることは必要条件としていない。
また、これはJSファイルであり、 js.src を修正すると同時に js ファイルを更新する必要がある。
補足¶
js.src から js ファイル(minimize js)を作成するのに JSMIN を用いているらしい。 jsmin.c をコンパイルし、実行時にリダイレクトでファイルを渡せば minimize js が出力できる。
$ ./jsmin.out < editor_plugin.js.src > editor_plugin.js
Related issues
Associated revisions
(fixed #2000) fixed cell of emoji palette to display 25 items per line
(fixed #2000) rewrote a conditional expression not to depend on emoji start-id
History
#1 Updated by Minoru Takai over 13 years ago
- Status changed from New(新規) to Accepted(着手)
- Assignee set to Minoru Takai
- 3.6 で発生するか set to Yes
#2 Updated by Minoru Takai over 13 years ago
- File deleted (
Screen_shot_2011-04-09_at_13.53.28のコピー.png)
#3 Updated by Minoru Takai over 13 years ago
- File emoji_palette.png View added
#4 Updated by Minoru Takai over 13 years ago
- Status changed from Accepted(着手) to Pending Review(レビュー待ち)
- % Done changed from 0 to 50
更新履歴 1deb834eb65ba32c6645ef26d7fe0325baf72ccc で適用されました。
#5 Updated by Naoya Tozuka about 13 years ago
- Status changed from Pending Review(レビュー待ち) to Rejected(差し戻し)
修正箇所確認しました。
ここで (i - 1) % 25 の 1 は emoji.start の値であり、emoji.start が別の値だった場合にはその値に差し替えることになりますので、マジックナンバー的にソースに埋め込むのは好ましくありません。(25という値もマジックナンバーだと思いますがここでは触れません)
for (var i = emoji.start; i <= emoji.end; i++) { if ((i - emoji.start) % 25) { tr = DOM.add(tb, 'tr'); } ...のようにするのが良いと思われます。
#6 Updated by Minoru Takai about 13 years ago
- Status changed from Rejected(差し戻し) to Accepted(着手)
note-5 を受けて修正します。確かに emoji.start が 1 であることを前提とした修正になっていました。 emoji の id に関わらず行当たり25個の絵文字が表示されるように if ((i - emoji.start) % 25) という条件式に書き換えることにします。
#7 Updated by Minoru Takai about 13 years ago
- Status changed from Accepted(着手) to Pending Review(レビュー待ち)
更新履歴 ccaf4813ac629002d13c20f541105e486d7b08d8 で適用されました。
#8 Updated by Naoya Tozuka about 13 years ago
- Status changed from Pending Review(レビュー待ち) to Pending Testing(テスト待ち)
- % Done changed from 50 to 70
修正確認しました。OKです。
#9 Updated by Shingo Yamada about 13 years ago
- 360対象 set to RC1
#10 Updated by Yuma Sakata about 13 years ago
- 360対象 deleted (
RC1)
#11 Updated by Yuma Sakata about 13 years ago
- 360対象 set to RC1
#12 Updated by Yuma Sakata almost 13 years ago
- Status changed from Pending Testing(テスト待ち) to Fixed(完了)
- % Done changed from 70 to 100
テストOKです。
#13 Updated by kaoru n about 9 years ago
- 3.8 で発生するか set to Unknown (未調査)
#14 Updated by kaoru n almost 8 years ago
- Related to Task(タスク) #4071: OpenPNEに同梱されている TinyMCE 3.4.4 が IE11 に対応していないため、バージョンアップについて調査する added