Ticket #414 (closed バグ指摘: 修正済)

Opened 12 years ago

Last modified 9 years ago

モバイル画像生成で高さの指定が不正になる

Reported by: kakinaka Owned by: somebody
Priority: Milestone: EC-CUBE2.4.0
Component: フロント Version: 2.4.0RC-2
Keywords: Cc:
修正済み: yes

Description

モバイル端末でモバイルページを閲覧すると、モバイル用の画像が自動生成されますが、
なぜか1つ目の画像の高さ情報を保持したままとなり、2枚目以降の画像が崩れる場合があります。

Change History

comment:1 Changed 12 years ago by Seasoft

  • Version changed from 2.3.3 to 2.4.0RC-2
  • Milestone changed from EC-CUBE2.3.4 to EC-CUBE2.4.0

フォーラムにて QAZU 様から実記での詳細な報告があり、当方ではシミュレータでも再現を確認しました。

 http://xoops.ec-cube.net/modules/newbb/viewtopic.php?topic_id=3905&forum=8

1ページに1つの画像しかない場合は問題ないんですが、
ヘッダーに画像を置き、タイトル画像や商品画像など複数の画像が存在する場合、画像の変換時に一番上の画像のサイズにリサイズされてしまいます。

表示したい画像の大きさが
ヘッダー画像[240*30]
タイトル画像[240*20]
商品画像[220*200]

だったとします。

しかし、実際携帯で確認すると全ての画像が[240*30]になってしまうんです。

QAZU 様は下記バージョンで発現しているようです。

  • 2.3.4

当方では下記バージョンで再現を確認しました。

  • コミュニティ
  • 2.4.0RC-2

comment:2 Changed 12 years ago by Seasoft

Masashige 様から、改善方法の投稿がありました。

 http://xoops.ec-cube.net/modules/newbb/viewtopic.php?viewmode=thread&topic_id=3905&forum=8&post_id=16449#forumpost16449

記事からすると、width の方も改善の余地があるのかもしれません。(もしや、勝手に拡大されるとか??)

以下、記事抜粋です。


if (is_null($this->outputImageHeight)) {
	$this->outputImageHeight = $inputImageHeight * ($this->outputImageWidth / $inputImageWidth);
}

image_converter.incの33~35行目ですね。2枚目以降の呼び出しで1枚目の情報が残ってしまうようで、ここがスルーされます。

面倒だったので(えー!?)33、35行目をコメントアウトして対応しました。どうせキャッシュされるし、納期も無茶だし、まぁいっか!みたいな。そんなんしてたらwidth固定とかも直したくなるし…。

comment:3 Changed 12 years ago by kishik

33行目以降を
if (is_null($this->outputImageHeight)) {

$this->outputImageHeight = $inputImageHeight * ($this->outputImageWidth / $inputImageWidth);

}

から

if (is_null($this->outputImageHeight)) {

$height_was_null = TRUE;
$this->outputImageHeight = $inputImageHeight * ($this->outputImageWidth / $inputImageWidth);

} else {

$height_was_null = FALSE;

}

に変えて、
(変更前の)118行目に
if ($height_was_null) {

$this->outputImageHeigh = NULL;

}
を挿入すれば、元のロジックを崩さなくていいはずです。

comment:4 follow-up: ↓ 5 Changed 12 years ago by Seasoft

QAZU 様から、Width を含めた解決方法の投稿がありました。

 http://xoops.ec-cube.net/modules/newbb/viewtopic.php?viewmode=thread&topic_id=3905&forum=8&post_id=16452#forumpost16452

ただし、320px で固定しているので、この部分は $this->outputImageWidth を使用する(要 NULL チェック?)のが適切な気がします。

comment:5 in reply to: ↑ 4 Changed 12 years ago by kishik

掲示板にも書きましたが、

QAZUさんの投稿分では解決できません。

使用している画像の横幅が320px以下だと

解決しているように見えるだけのはずです。

コメント3つ目に書いた方法が正しいはずです。

comment:6 Changed 12 years ago by kishik

画像が最大携帯画像サイズ以下の場合には

そのままの大きさのものを表示するように

コードを以下のものに訂正します。

現在のEC-CUBEは横幅しか考慮されていませんが、

高さも制限に入れるようにした場合でも対応できるようにしたつもりです。

if (is_null($this->outputImageHeight)) {
  $height_was_null = TRUE;
  $this->outputImageHeight = $inputImageHeight * ($this->outputImageWidth / $inputImageWidth);
} else {
  $height_was_null = FALSE;
}
if ($inputImageWidth <= $this->outputImageWidth) {
  if ($inputImageHeight <= $this->outputImageHeight) {
    $this->outputImageWidth  = $inputImageWidth;
    $this->outputImageHeight = $inputImageHeight;
  } else {
    $this->outputImageWidth = $inputImageWidth * ($this->outputImageHeight / $inputImageHeight);
} else {
  if ($inputImageHeight <= $this->outputImageHeight) {
    $this->outputImageHeight = $inputImageHeight * ($this->outputImageWidth / $inputImageWidth);
  } else {
    if ($this->outputImageWidth / $inputImageWidth < $this->outputImageHeight / $inputImageHeight) {
      $this->outputImageHeight = $inputImageHeight * ($this->outputImageWidth / $inputImageWidth);
    } else {
      $this->outputImageWidth = $inputImageWidth * ($this->outputImageHeight / $inputImageHeight);
    }
  }
}

comment:7 Changed 12 years ago by kishik

携帯各端末での画像高さを指定しても縦横比を崩さずに

最適な大きさにリサイズされるようになったはずなので、

/data/class/SC_MobileImage.php

の60行目に

$imageHeight = $data[4];

を追加して、(追加前の)86行目に

$imageConverter->setImageHeight($imageHeight);

も追加すれば、

/data/include/mobile_image_map_*.csv

が携帯サイトでの(縦横を含めた)最大画像サイズのデータを表すようになり、

ユーザーの意図する動作に近くなると思います。

2.4.0での導入をご検討ください。

comment:8 Changed 12 years ago by kishik

追記です。

その場合、

/data/include/image_converter.inc

の128行目に

function setImageHeight($imageHeight) { $this->outputImageHeight = $imageHeight; }

の挿入も必要でした。

確認不足失礼しました。

comment:9 Changed 12 years ago by kajiwara

本件、いろいろご調査いただきありがとうございます。

kishik様の修正方法にて修正させていただきました。
r18006 にて対応いたしました。)

不具合など特にないようでしたら2.4.0正式版に含めたいと思います。
よろしくお願いいたします。

comment:10 Changed 12 years ago by kishik

2009/05/12 12:15:28

で記述している、

(変更前の)118行目に

if ($height_was_null) {

$this->outputImageHeigh = NULL;

}

の処理が足りないので、

これも追加お願いします。

comment:11 Changed 12 years ago by kajiwara

r18009 kishik様のご指摘通り、不足分のコードを追加させていただきました。

comment:12 Changed 12 years ago by kajiwara

  • Status changed from new to closed
  • Resolution set to 修正済

comment:13 Changed 11 years ago by Seasoft

  • Status changed from closed to reopened
  • Resolution 修正済 deleted

追記したソースの整形が open_trac/wiki/EC-CUBE標準規約 に従わず、TAB & 2SP という不可解な状態になっています。

抜粋

インデントは, 半角スペース4文字を使用し, タブは使用しない. 

元々が TAB になっているようですが、これも 4SP に変換しても良い気がします。秀丸エディタなら、TAB → SP の変換はスマートに可能です。参考まで。

comment:14 follow-up: ↓ 15 Changed 11 years ago by kajiwara

Seasoft様 ご指摘ありがとうございます。

r18145r18146 にてインシデントがtabの部分、半角空白(×2)となっている部分を全て半角空白(×4)に修正させていただきました。

※本修正(r18145r18146)は、ロジックに関係なく、また#414の修正の本筋とは関係ない部分も含まれておりますので、必須のマージ項目ではございません。
#414の修正内容に関しては、r18006r18009 をご参照いただければと思います。

comment:15 in reply to: ↑ 14 Changed 11 years ago by Seasoft

  • Status changed from reopened to closed
  • Resolution set to 修正済

インデントの修正を確認しました。

comment:16 Changed 10 years ago by Seasoft

コミュニティ r18085 r18086 r18305 r18306

comment:17 Changed 9 years ago by Seasoft

  • 修正済み unset

このチケットでの実装には不備が確認されています。

#1376 で改修を行っています。

comment:18 Changed 9 years ago by Seasoft

  • 修正済み set

意図せず、修正済みが削除されたので、戻します。

Note: See TracTickets for help on using tickets.