Bug（バグ） #2441
「携帯版フォントサイズ指定設定」が反映されない箇所がある
0%
説明
関連するチケット
関係しているリビジョン
(fixes #2441) fixed regex of convertAddFont4Docomo() for a config of setting font-size on the mobile_frontend.
履歴
#1
Kiwa Sakai が12年以上前に更新
- 説明 を更新 (diff)
#2
Shingo Yamada が12年以上前に更新
- 担当者 を Minoru Takai にセット
#3
Minoru Takai が12年以上前に更新
- 対象バージョン を OpenPNE 3.7.0 にセット
master ブランチにおいてこの問題を対応します。安定版へのバックポートについては master ブランチ対応後の次期マイナーバージョンにおいて対応します（3.6.0 では対応しません）。
#4
Minoru Takai が12年以上前に更新
- ステータス を New（新規） から Accepted（着手） に変更
概要¶
まず、「携帯版フォントサイズ指定設定」というものが何なのかを書いておく（このチケットを見た時点では何のことか知らなかった）。
- 管理画面 > SNS設定 > 携帯関連設定 > 携帯版フォントサイズ指定設定
この設定項目があり、設定値は、
- フォントサイズを指定する
- フォントサイズを指定しない
とある。管理画面上でこれを読んでも何が起きるのか全く推測できないが、どうやらこれを「指定する」にすると携帯版で文字が小さめに表示されるようになるらしい（動作からの推測）。
しかしこれが一部で機能していない、という問題である。具体的にどこで機能していないかについては調べ切れていないが、後述する原因による箇所としてはマイホームのインフォメーションボックスが挙げられる。
このチケットでの対応範囲¶
docomo 端末について、後述する原因によって生じている箇所のみを対応しています。これ以外の原因で本問題が生じている箇所があれば問題報告者として改めて示してください（テスターとして動作テスト時に差し戻して頂いても構いません）。
携帯版フォントサイズ指定設定の仕様確認¶
この設定値の意味を調べてみた。
3系にこの機能が実装されたのは c68e4a70 のようである。チケットは http://trac.openpne.jp/ticket/3313 で、 http://trac.openpne.jp/changeset/10143 の修正が行なわれている。
ここまで読んでも、この設定項目の意味が分からない。2系にあった機能を3系にも付けるという経緯らしいので、2系の管理画面を見てみた。
携帯版でのフォントサイズ指定をおこなうかどうかを設定します * フォントサイズを指定する／指定しない（※ここはフォームコントロール） ※「フォントサイズを指定する」を選択した場合、文字サイズが小さめに設定されます ※「フォントサイズを指定しない」を選択した場合、携帯端末ごとのデフォルトの文字サイズで表示されるようになります
ここまで読んでやっとこの設定値の意味が分かった。
調査¶
http://trac.openpne.jp/changeset/10143 では、前述の仕様に対して、実装上は「フォントサイズを指定する」設定になっている場合、
apps/mobile_frontend/templates/layout.php
05 <?php if ($sf_request->getMobile()->isSoftBank() && $op_config['font_size']): ?>
06 <style type="text/css">
07 *{font-size:small;}
08 </style>
09 <?php elseif ($sf_request->getMobile()->isEZWeb() && $op_config['font_size']): ?>
10 <style type="text/css">
11 *{font-size:xx-small;}
12 </style>
13 <?php endif; ?>
という記述にある style 要素の出力が行なわれ、 softbank と ezweb の端末において、フォントサイズが小さめになるようにしている。
しかしこれでは docomo 端末でこの機能が提供されていないとして、次のチケット http://redmine.openpne.jp/issues/668 で対応された。 docomo 端末である場合は、文字サイズが小さくなるべき部位に size 属性を指定した font 要素を追加するという処理になっている。
今回問題が生じていたのは、この後半の docomo 端末に対しての機能であり、 #668 での修正内容の一部が適切ではなかったと考えられるものである。
原因は、 font 要素を追加する箇所を特定する正規表現が、特定したい箇所にマッチしないことである。
lib/filter/sfMobileIOFilter.class.php
143 private function convertAddFont4Docomo()
144 {
145 $response = $this->getContext()->getResponse();
146 $content = $response->getContent();
147 $pattern_start_tag = array('/(<body.*?>)/', '/(<td.*?>)/');
148 $replacement_start_tag = '$1<font size="2">';
149 $pattern_end_tag = array('</body>', '</td>');
150 $replacement_end_tag = array('</font></body>', '</font></td>');
151
152 $content = preg_replace($pattern_start_tag, $replacement_start_tag, $content);
153 $content = str_replace($pattern_end_tag, $replacement_end_tag, $content);
154 $response->setContent($content);
155 }
156 }
147 行目では /(<body.*?>)/ などと記述しているが、 body 要素開始タグは、これにマッチしない形で出力されている可能性がある。 td 要素についても同様である。
今回、実際に問題になっているのは恐らく body 要素タグに関してであり、以下のとおり "<body" と ">" の間に（任意の属性と共に）改行が含まれており、上記の正規表現ではマッチしていない。
apps/mobile_frontend/templates/layout.php 16 <body 17 <?php if (!($sf_request->getMobile()->isDocomo() && '#000000' === $op_color['core_color_14'])): ?> 18 text="<?php echo $op_color['core_color_14'] ?>" 19 <?php endif; ?> 20 bgcolor="<?php echo $op_color['core_color_1'] ?>" 21 link="<?php echo $op_color['core_color_15'] ?>" 22 <?php if ($sf_request->getMobile()->isDocomo()): ?> 23 alink="<?php echo $op_color['core_color_23'] ?>" 24 <?php endif; ?> 25 vlink="<?php echo $op_color['core_color_17'] ?>" 26 >
修正内容¶
body 要素タグ、および td 要素タグについて、改行が含まれていたりする場合についてもマッチするように、 sfMobileIOFilter クラスの convertAddFont4Docomo() メソッド（の正規表現部分）を書き換える。
diff --git a/lib/filter/sfMobileIOFilter.class.php b/lib/filter/sfMobileIOFilter.class.php
index 6bf6ed1..ce11de8 100644
--- a/lib/filter/sfMobileIOFilter.class.php
+++ b/lib/filter/sfMobileIOFilter.class.php
@@ -144,13 +144,16 @@ class sfMobileIOFilter extends sfFilter
{
$response = $this->getContext()->getResponse();
$content = $response->getContent();
- $pattern_start_tag = array('/(<body.*?>)/', '/(<td.*?>)/');
- $replacement_start_tag = '$1<font size="2">';
- $pattern_end_tag = array('</body>', '</td>');
- $replacement_end_tag = array('</font></body>', '</font></td>');
- $content = preg_replace($pattern_start_tag, $replacement_start_tag, $content);
- $content = str_replace($pattern_end_tag, $replacement_end_tag, $content);
+ $elements = array('body', 'td');
+ $elementsExpr = implode('|', $elements);
+ $pattern = array(
+ '#(<(?:'.$elementsExpr.')(?:\s[^>]*)?>)#i',
+ '#(</(?:'.$elementsExpr.')(?:\s[^>]*)?>)#i'
+ );
+ $replacement = array('$1<font size="2">', '</font>$1');
+
+ $content = preg_replace($pattern, $replacement, $content);
$response->setContent($content);
}
}
修正内容の解説¶
1: $elements = array('body', 'td');
2: $elementsExpr = implode('|', $elements);
3: $pattern = array(
4: '#(<(?:'.$elementsExpr.')(?:\s[^>]*)?>)#i',
5: '#(</(?:'.$elementsExpr.')(?:\s[^>]*)?>)#i'
6: );
7: $replacement = array('$1<font size="2">', '</font>$1');
8:
9: $content = preg_replace($pattern, $replacement, $content);
==== 2行目の時点での値
$elementsExpr = 'body|td'
==== 4行目の正規表現：開始タグ
#(<(?:'.$elementsExpr.')(?:\s[^>]*)?>)#i
ここでの # は s/// を s### と書くことがあるように、
正規表現リテラルを作るための記号である。
内容は次の通り：
(< // < から始まり
(?:'.$elementsExpr.') // body|td が続き
(?:\s[^>]*)? // 何も無いか、空白類に続けて属性など（※）があり
>) // > で終わる
※ [^>]* で「属性など」を表しているが、
これには > 以外の文字（空白や改行、属性、不正な文字など）が含まれる。
全体を () で括りキャプチャしている。 $replacement 側で $1 で参照している。
i オプションを付けており、要素名 (body, td) の大文字小文字を区別していない。
== 5行目の正規表現：終了タグ
#(</(?:'.$elementsExpr.')(?:\s[^>]*)?>)#i
内容は次の通り：
(</ // </ から始まり
(?:'.$elementsExpr.') // body|td が続き
(?:\s[^>]*)? // 何も無いか、空白類に続けて文字（※）があり
>) // > で終わる
開始タグと同様にしている。
※ 空白類の連続であれば妥当だが、不正な文字がある場合もマッチする。
- <body*> の開始タグについて、 * の部分に改行があってもマッチするようにした
- 終了タグについても </body> だけでなく </body*> にマッチするようにした
- タグ名を前方一致ではなく完全一致に変更した（「空白類に続けて」を追加した）
- /(<body.*?>)/ を /<body(\s[^>]*)?>/ のようにした
- 要素名の大文字小文字を区別しないようにした
- その他
- コーディング規約違反であるアンダースコア区切りの変数を削除した
- preg_replace(), str_replace() と別々に呼んでいたのを preg_replace() だけにした
- 置換に正規表現を用いない前提であれば修正前の方が効率は良いと思われるが、終了タグについても正規表現を用いることにしたため preg_replace() を開始タグ・終了タグのそれぞれで実行することになる
- ただし、 body, td の2要素について別々に走査していたのを body|td として同時に走査するようにしているため、この点では修正後の方が効率は良いと思われる。
- べた書きではないため柔軟性はないが $elements の要素リストに対して font 要素タグを追加するようにして保守性を向上させた
#5
Minoru Takai が12年以上前に更新
- ステータス を Accepted（着手） から Pending Review（レビュー待ち） に変更
- 進捗率 を 0 から 50 に変更
更新履歴 effbc1310939d030ccaea8af93bdf3c8a198bc15 で適用されました。
#6
Minoru Takai が12年以上前に更新
全体を () で括りキャプチャしている。 $replacement 側で $1 で参照している。
などと書きましたが、よく考えたら全体は $0 で参照できますね。ここはもともと（全体を明示的にキャプチャして） $1 で参照するように書かれていたのでそのまま書いてしまっていました。
これについて改めて考えましたが、明示的にキャプチャして $1 で参照することが（ $0 を使うよりも）適切でないというわけでもないですし、この部分の処理はたまたまマッチした全体を置換する記述になっているだけであって、置換したい箇所を $1 で参照できる（明示的にキャプチャしている）ことにも僅かながらメリットがあるように思います。（とはいっても結果的に全体を参照したいなら $0 で済むわけですが。）
コメントのみ残しておきます。
#7
Shouta Kashiwagi が約12年前に更新
- 対象バージョン を OpenPNE 3.7.0 から 252 に変更
#8
Shouta Kashiwagi が約12年前に更新
- 対象バージョン を 252 から OpenPNE 3.8.x に変更
#9
isao sano がほぼ10年前に更新
- コピー先 Backport（バックポート） #3668: 「携帯版フォントサイズ指定設定」が反映されない箇所がある を追加
#10
Shinichi Urabe が9年以上前に更新
- ステータス を Pending Review（レビュー待ち） から Rejected（差し戻し） に変更
- 3.6 で発生するか を Unknown (未調査) にセット
- 3.8 で発生するか を Unknown (未調査) にセット
- タグの閉じるまでの
(?:\s[^>]*)?>ですが、\s空白は[^>]に含まれるので不要、また、また、>の前の文字は[^>]で>以外の文字の連続という表現のため、最短マッチを示す?は不要です\sを削除することにより、>の直前にある 0 回または 1 回の繰り返しを示す?は*が 0 回以上の繰り返しを示すため、不要となり、[^>]自体を括弧で囲む必要がなくなります - 正規表現の修正はできればユニットテストがあるとよいかと思います
#11
Shinichi Urabe が9年以上前に更新
- ステータス を Rejected（差し戻し） から Pending Testing（テスト待ち） に変更
- 進捗率 を 50 から 70 に変更
Shinichi Urabe は書きました:
- タグの閉じるまでの
(?:\s[^>]*)?>ですが、\s空白は[^>]に含まれるので不要、また、また、>の前の文字は[^>]で>以外の文字の連続という表現のため、最短マッチを示す?は不要です\sを削除することにより、>の直前にある 0 回または 1 回の繰り返しを示す?は*が 0 回以上の繰り返しを示すため、不要となり、[^>]自体を括弧で囲む必要がなくなります
body|td の後は "空白類に続けて" というところがポイントで、\s を削除してしまうと、文字列が続いた場合もマッチしてしまうので、上記差し戻しの指摘は不適切でした
テスト待ちに変更します
#12
誠二 天重 が9年以上前に更新
- コピー先 Backport（バックポート） #3688: 「携帯版フォントサイズ指定設定」が反映されない箇所がある を追加
