Project

General

Profile

Bug(バグ) #3560

スマートフォンで画像をアップロードすると画像が横向きになってしまう場合がある

Added by Mutsumi Imamura over 5 years ago. Updated over 2 years ago.

Status:
Won't fix(対応せず)
Priority:
Normal(通常)
Assignee:
-
Target version:
Start date:
2014-02-21
Due date:
% Done:

0%

3.6 で発生するか:
Yes (はい)
3.8 で発生するか:
Yes (はい)

Description

Overview (現象)

スマートフォンで画像をアップロードすると画像が横向きになってしまう場合がある

再現バージョン

  • OpenPNE3.8.10
  • OpenPNE3.6.13

再現手順

(再現方法をここに記述)

Causes (原因)

スマートフォン等で撮影した画像にはexif情報が埋め込まれます。
exif情報には、撮影したカメラの機種や撮影時の条件、撮影方向等が載っています。
iPhone等のスマートフォンでは、画像を表示する際にexif情報を読み取ることで最適な画像の向きで表示できるようになっています。
しかしOpenPNEでは画像を表示する際にexif情報を読みとらずにそのまま表示されたことから「縦と横が反転して、横向きに画像がアップされる」という現象が発生します。

Way to fix (修正内容)

修正内容を記入


Related issues

Copied to OpenPNE 3 - Backport(バックポート) #3562: スマートフォンで画像をアップロードすると画像が横向きになってしまう場合がある Fixed(完了) 2014-02-21
Copied to OpenPNE 3 - Backport(バックポート) #3564: スマートフォンで画像をアップロードすると画像が横向きになってしまう場合がある Fixed(完了) 2014-02-21

Associated revisions

Revision 5d3ca7b9
Added by Yuya Watanabe over 5 years ago

Merge remote-tracking branch 'origin/pr/107' (fixes #3560)

History

#1 Updated by Mutsumi Imamura over 5 years ago

  • Description updated (diff)
  • 3.6 で発生するか changed from Unknown (未調査) to Yes (はい)
  • 3.8 で発生するか changed from Unknown (未調査) to Yes (はい)

#2 Updated by Mutsumi Imamura over 5 years ago

#3 Updated by Mutsumi Imamura over 5 years ago

#4 Updated by Yuya Watanabe over 5 years ago

対象 pull request

able to read exif data and orient the direction to repair of the uploade... by AkihiroKOBAYASHI · Pull Request #103 · openpne/OpenPNE3

https://github.com/openpne/OpenPNE3/pull/103

#5 Updated by Yuya Watanabe over 5 years ago

  • Status changed from New(新規) to Pending Fixing(修正待ち)

修正コミットが見当たらないです.

#6 Updated by Yuya Watanabe over 5 years ago

修正コミットは f20e9a3 だと思いますがマージやコンフリクトの解消時に削除されているようです.最終的なマージコンフリクトの解消は master マージ時に行いますので本チケットの修正に則した内容のコミットのみのプルリクエストでお願いします.

b202426 able to resize the uploaded image when its size is larger than assigned.
28047ed start
4ad8194 able to resize the uploaded image when its size is larger than assigned.
dc57650 start
d897a1d Merge branch 'master' of https://github.com/AkihiroKOBAYASHI/OpenPNE3
30fb274 Merge branch 'master' of https://github.com/AkihiroKOBAYASHI/OpenPNE3
29cecef able to compress the uploaded image when its size is larger than assigned size
3b8dc51 able to resize the uploaded image when the size of it is larger than that of assigned
f20e9a3 able to read exif data and orient the direction to repair of the uploaded image

#7 Updated by Yuya Watanabe over 5 years ago

対象 pull request

able to read exif data and orient the direction to repair of the uploaded image by AkihiroKOBAYASHI · Pull Request #105 · openpne/OpenPNE3
https://github.com/openpne/OpenPNE3/pull/105

#8 Updated by Yuya Watanabe over 5 years ago

  • Status changed from Pending Fixing(修正待ち) to Accepted(着手)

#9 Updated by Yuya Watanabe over 5 years ago

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

#10 Updated by Yuya Watanabe over 5 years ago

  • Status changed from Pending Review(レビュー待ち) to Rejected(差し戻し)

フィードバック

すべて save() メソッドの中に記述されていますが,この辺りは「Orientation にしたがって画像を回転する」という明確な目的の一連の流れが存在するため,名前付けしてメソッドとして抽出してもらったほうがよいです.

メソッド抽出の具体的なメリットは リファクタリングという書籍の 「Extract Method(メソッドの抽出)」の項目が参考になります.

本チケットにおけるメソッド抽出の利点としては #3561 の修正と同時にマージする際に save() 内でのコンフリクトの解消時に ロジック記述部分を気にしたマージを行う必要が生じるものを,メソッド抽出後は save() 内ではメソッドの追加のみで対応できるというようなことがあります.

実際にマージを行う場合では画像変更処理はまとめておいたほうがよいと思われるためメソッドに抽出した処理については共通化する可能性はありますが,処理単位でまとまっているほうが共通化なども行いやすくなります.

#11 Updated by Mutsumi Imamura over 5 years ago

  • Status changed from Rejected(差し戻し) to Pending Review(レビュー待ち)

https://github.com/openpne/OpenPNE3/pull/107 でプルリクエストが来ています

#12 Updated by Yuya Watanabe over 5 years ago

  • Status changed from Pending Review(レビュー待ち) to Rejected(差し戻し)

だいぶよくなったと思います.以下簡単にフィードバック.
ちなみに github 側の pull request についてですが,今の段階ではコミットを一つにまとめる必要はないです.
この段階で一つにまとめてしまうとフィードバックの修正内容が最初のコミットからどのように反映されているか,あるいはそもそも反映されているかどうかが追いにくいためです.あと,念のため先んじて書いておきますが「fixed for feedback」のような修正内容がわからないコミットメッセージは後々コードを git log や git blame などで追う際に重大な情報量不足となるため控えてもらうと助かります.

フィードバック1

アクセス修飾子がすべて public になっていますが外側で使うタイミングがなさそうなメソッドのため private または protected のほうが良いです.ここの場合, File オブジェクトは部分集合として画像を持ち,その部分集合の要素である場合に必要なメソッドであるため外側のクラスからの呼び出しで用いられることは望ましくなく private で良いと思います. public や protected にするならば画像または画像変換ロジックを扱うクラスで行ったほうがよさそうです.
一般的にプロパティやメソッドのスコープは必要でない限り小さいほうが良いというスタンスが多いと思います.具体的な例は検索してもらうとよいと思います.

フィードバック2

createOrientedImage() 内の case 文で 3, 6, 8 などマジックナンバーが用いられています.今の時点ではコードを見ればまあわかりますが,より早く処理の概要を捉える目的やコードが肥大化した場合に処理が追いにくくなること防ぐ目的のために回転方向をコメントで追記してもらうと良いです.またコーディング規約には記述されていませんがコメントで単語や文章を用いる際には英語でお願いします.

フィードバック3

コーディング規約について

http://www.openpne.jp/coding-standards-ja/#id28

クラスと同様、波括弧は関数名の次に書かなければなりません。関数名と引数定義用の開始括弧の間にはスペースを入れてはいけません。

http://www.openpne.jp/coding-standards-ja/#id34

変数と値を比較する際は最初に値を置き、場合によっては型チェックもおこなってください:

if (1 === $var)

#14 Updated by Yuya Watanabe over 5 years ago

  • Status changed from Rejected(差し戻し) to Accepted(着手)
  • % Done changed from 50 to 0

#15 Updated by Yuya Watanabe over 5 years ago

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

#16 Updated by Yuya Watanabe over 5 years ago

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

対応有難うございました.大丈夫と思うためマージします.

#17 Updated by Yuya Watanabe over 5 years ago

ちなみにコミットメッセージが 二つのコミットで全く同じとなっていますが,まあ大丈夫とします.
修正内容としては note-12 の 3 つとなるためコミットもそれぞれの目的に合わせてコミットメッセージを変更し,わかりやすいコミットであるほうが望ましいと思います.ここではまあ大丈夫とします.

#18 Updated by Akihiro KOBAYASHI over 5 years ago

@nabe 了解です。以後気をつけます。ただ、#3561も二つのコミットで全く同じとなっていると思うので・・・・・・そこはお許しください

#19 Updated by Yuya Watanabe over 5 years ago

5d3ca7b953f42e04462aefeede9862233904410a で master にマージしました.

#20 Updated by Yuya Watanabe over 5 years ago

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

更新履歴 5d3ca7b953f42e04462aefeede9862233904410a で適用されました。

#21 Updated by Mutsumi Imamura over 5 years ago

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

レビュー待ちに戻ってしまっているのでステータス変更します。

#23 Updated by kaoru n over 2 years ago

  • Status changed from Pending Testing(テスト待ち) to Won't fix(対応せず)
  • Target version changed from OpenPNE 3.9.0-old to OpenPNE 3.9.0
  • % Done changed from 70 to 0

OpenPNE 3.8.11 にて対応済みであったため、対応せずとします。

Also available in: Atom PDF