Project

General

Profile

Bug(バグ) #2418

opSkinClassicPlugin を使うと pc_frontend のナビゲーションでクリックできない箇所がある

Added by Fumie Toyooka almost 8 years ago. Updated over 3 years ago.

Status:
Fixed(完了)
Priority:
Normal(通常)
Assignee:
Target version:
Start date:
2011-09-22
Due date:
% Done:

100%

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

Description

概要

OpenPNE にはグローバルナビ、ローカルナビがある。これは3系と2系両方に存在するが、3系と2系では項目が異なっている。

opSkinClassicPlugin は、3系で2系のデザインを模倣するスキンだが、これを用いると2系のナビ項目の名称が表示されてしまい、次の問題が生じる。

  • (P1) 2系にしか存在しない(3系で実装されていない)ナビ項目が機能しない(クリックできない項目が表示されている)
  • (P2) 3系にしか存在しないナビ項目が表示できない(項目名が2系で用いている画像で表示されているため)

これとは別に、次の問題が生じている。

  • (P3) 3系に存在するナビ項目のほとんどが機能していない(クリックできない)

このチケットでは (P3) を扱う。

補足

opSkinClassicPlugin を用いなければよいという話ではあるが、2系からのコンバートを行ったSNSはデフォルトで opSkinClassicPlugin が使われる。これを加味するとこの問題がどれほど重大であるかが考えやすいかもしれない。

バージョン

  • OpenPNE-3.6.x で生じている(ログアウトなど基本機能がクリックできない)
    • stable-3.6.x ブランチ
    • master ブランチ

OpenPNE-3.4.x では、ログアウトなどの基本機能はクリックできる。しかし 3.4.x でも、実装されているがクリックできないナビ項目が存在するかもしれない(後述の原因を参照)。

原因

この問題は主に次の 3 つの原因によるものである。

  • (C1) opSkinClassicPlugin の CSS に記述されているセレクタと、実際に出力される HTML 上の id 値が一致していない(3.6.x 以降)
  • (C2) 3系で新たに実装された機能(バンドルされたプラグインで実装された機能)に対応するナビ項目について、そのナビ項目を機能させるために opSkinClassicPlugin の CSS を書き換える(追記する)必要があるが、それを行なっていない(3.6.x 以降、もしかしたら 3.4.x でもあるかもしれない)
  • (C3) ナビゲーション項目の id 値を決定するための op_url_to_id() 関数が URL の記述の仕方(member/search か @member_search かの違い)によって、出力される値が異なっている( #257 時点からで、これは 3.3.0 対象なので、 3.4.x 以降)

修正方針

原因の (C2) については CSS に追記する対応をする。

(C1) について、 3.6.x 以降で生じているが 3.4.x では生じていない点については #1004 の修正(3.5.2 対象)がきっかけであり、この問題の根本的な原因は #2573da8aa22 で定義されている op_url_to_id() の定義 か使い方 ( glovalNav / localNav ) が誤っていることである。

(C1) については次のいずれかの方針で修正する。

  • (S1): HTML 側の出力は修正せずに CSS 側のセレクタを修正する
  • (S2): HTML 側の出力(恐らく op_url_to_id() 関数の定義)を修正する
    • この場合、既に CSS ( 3.4.x および 3.6.x 以降)に _foo と __bar のように (C3) の問題を受けた記述が存在するため、 CSS も併せて修正する必要がある

Related issues

Related to OpenPNE 3 - Backport(バックポート) #2438: opSkinClassicPlugin を使うと pc_frontend のナビゲーションでクリックできない箇所がある Won't fix(対応せず) 2011-09-26 2012-01-12
Related to OpenPNE 3 - Backport(バックポート) #2475: opSkinClassicPlugin を使うと pc_frontend のナビゲーションでクリックできない箇所がある Fixed(完了) 2011-10-06

Associated revisions

Revision 582fc718 (diff)
Added by Minoru Takai over 7 years ago

(fixes #2418) modified CSS of nav selectors in opSkinClassicPlugin

Revision 8cd31b77 (diff)
Added by Minoru Takai over 7 years ago

Revert "(fixes #2418) modified CSS of nav selectors in opSkinClassicPlugin"

This reverts commit 582fc7187a88f580d326df644394b6d4ea903dc6.

Revision 05a1c7fa (diff)
Added by Minoru Takai over 7 years ago

(fixes #2418) changed op_url_to_id() and nav templates to fix nav id format, and modified CSS of nav selectors in opSkinClassicPlugin.

History

#1 Updated by Mutsumi Imamura over 7 years ago

  • 360対象 set to 3.6.0

#2 Updated by Fumie Toyooka over 7 years ago

  • Description updated (diff)

#3 Updated by Shingo Yamada over 7 years ago

  • Assignee set to Minoru Takai

#4 Updated by Mutsumi Imamura over 7 years ago

  • Subject changed from 2.14.x → 3.6.x へバージョンアップ後のグローバルナビでクリックできない箇所がある to opSkinClassicPluginにするとpc_frontendのナビゲーションでクリックできない箇所がある

他にクリック出来ない箇所が無いか、軽く調査しました。
クリック出来ない箇所についての調査結果は以下のとおりです。

グローバルナビ

  • マイホームと最新日記以外すべて

ローカルナビ

  • ホーム、日記、メッセージ、あしあと以外すべて

フレンドホームのナビ

  • 日記を読む、メッセージを送る以外すべて

コミュニティのナビ

  • 掲示板以外すべて

#5 Updated by Mutsumi Imamura over 7 years ago

  • Description updated (diff)

#6 Updated by Minoru Takai over 7 years ago

  • Status changed from New(新規) to Accepted(着手)
  • global
    <div id="globalNav">
    <ul>
    <li id="globalNav__homepage"><a href="/">マイホーム</a></li>
    <li id="globalNav__member_search"><a href="/member/search">メンバー検索</a></li>
    <li id="globalNav__community_search"><a href="/community/search">コミュニティ検索</a></li>
    <li id="globalNav__ranking"><a href="/ranking">ランキング</a></li>
    <li id="globalNav__album_list"><a href="/album">アルバム</a></li>
    <li id="globalNav_diary_index"><a href="/diary">日記</a></li>
    <li id="globalNav_blog_index"><a href="/blog">最新Blog</a></li>
    <li id="globalNav__member_config"><a href="/member/config">設定変更</a></li>
    <li id="globalNav__member_invite"><a href="/invite">友人を招待する</a></li>
    <li id="globalNav__member_logout"><a href="/logout">ログアウト</a></li>
    
    </ul>
    </div><!-- globalNav -->
    
  • local - myhome
    <div id="localNav">
    <ul class="default">
    <li id="default__homepage"><a href="/">ホーム</a></li>
    <li id="default__friend_list"><a href="/friend/list">マイフレンド</a></li>
    <li id="default_message_index"><a href="/message">メッセージ</a></li>
    <li id="default_ashiato_list"><a href="/ashiato/list">あしあと</a></li>
    <li id="default__application"><a href="/application">アプリ</a></li>
    <li id="default__album_list_mine"><a href="/album/listMember">アルバム</a></li>
    <li id="default_diary_listMember"><a href="/diary/listMember">日記</a></li>
    <li id="default_favorite_list"><a href="/favorite/list">お気に入り</a></li>
    <li id="default__member_profile_mine"><a href="/member/profile">プロフィール確認</a></li>
    <li id="default__member_editProfile"><a href="/member/edit/profile">プロフィール編集</a></li>
    </ul>
    </div><!-- localNav -->
    
  • local - friend
    <div id="localNav">
    <ul class="friend">
    <li id="friend__member_profile"><a href="/member/2">ホーム</a></li>
    <li id="friend__album_list_member"><a href="/album/listMember/2">アルバム</a></li>
    <li id="friend_diary_listMember"><a href="/diary/listMember/2">日記</a></li>
    <li id="friend__friend_list"><a href="/friend/list?id=2">フレンドリスト</a></li>
    <li id="friend_favorite_add"><a href="/favorite/add?id=2">お気に入りに追加</a></li>
    <li id="friend__obj_member_introfriend"><a href="/introfriend/2">紹介文を書く</a></li>
    <li id="friend_message_sendToFriend"><a href="/message/sendToFriend/id/2">メッセージを送る</a></li>
    <li id="friend__application_list"><a href="/application/list/2">アプリ</a></li>
    </ul>
    </div><!-- localNav -->
    
  • local - community
    <div id="localNav">
    <ul class="community">
    <li id="community__community_home"><a href="/community/9">コミュニティトップ</a></li>
    <li id="community_communityTopic_listCommunity"><a href="/communityTopic/listCommunity/9">トピックリスト</a></li>
    <li id="community_communityEvent_listCommunity"><a href="/communityEvent/listCommunity/9">イベントリスト</a></li>
    <li id="community__community_join"><a href="/community/join?id=9">コミュニティに参加</a></li>
    <li id="community__community_quit"><a href="/community/quit?id=9">コミュニティを退会</a></li>
    </ul>
    </div><!-- localNav -->
    

#7 Updated by Minoru Takai over 7 years ago

  • Subject changed from opSkinClassicPluginにするとpc_frontendのナビゲーションでクリックできない箇所がある to opSkinClassicPlugin を使うと pc_frontend のナビゲーションでクリックできない箇所がある
  • Description updated (diff)

#8 Updated by Minoru Takai over 7 years ago

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

更新履歴 582fc7187a88f580d326df644394b6d4ea903dc6 で適用されました。

#9 Updated by Minoru Takai over 7 years ago

note-8 のコミットを行った後に調査して note-7 で書き換えたチケットの内容を認識しました。

このコメントを行なっている時点で、本件 (C1) に対して (S1) で対応した note-8 の修正が適切であるとは思っていません(追記した (C2) の対応は適切なものです)。しかしながら 3.6.0 でも対応を行う都合上、どのような対応に落ち着かせるかを検討する必要があります。

note-8 の修正(これは (S1) の修正方針)について、この修正でよいと判断できるかレビューをお願いします。あるいは (S2) の修正を行うべきだという意見、またはそれ以外の見解があればコメントをお願いします。

  • (S1): HTML 側の出力は修正せずに CSS 側のセレクタを修正する
    • 【懸念点】 OpenPNE をノンカスタマイズ(ナビゲーション設定でナビの URL を書き換えていない状態)で opSkinClassicPlugin を使用している場合には note-8 の対応でとりあえず「クリックできるようになる」が、 op_url_to_id() 関数が URL の記述の仕方(member/search か @member_search かの違い)によって出力される id のアンダースコアの数が異なっている問題が残る。
  • (S2): HTML 側の出力(恐らく op_url_to_id() 関数の定義)を修正する(関数定義は変えないで、テンプレート側で、「先頭に _ が付いていたら除去する」といった対応も可能だろうか)
    • 【必要な修正】 3.4.x 以前( #257 時点)から、もともと @homepage のようにナビゲーション設定で URL を記述していた部分は、CSS上でも li#default__homepage と記述しているようなので、もともと CSS にアンダースコアを連続して記述していた部分を書き換える必要がある。
    • 【懸念点】 これまでにカスタムCSSなどを記述しているとセレクタがマッチしなくなるためにレイアウトが崩れたりする SNS が現れる可能性がある。これは SNS 管理者などに対応してもらう必要があるため、この問題についてブログ告知するなりして周知する必要がある。
      • li 要素の id 属性値に default__homepage と default_homepage の両方を指定しておくといったことで対応するのは本質的ではないし(特に id 属性なので文法違反であり対応不可能)、機械的に CSS 側のセレクタを解釈し直すことも現実的ではなく、特に SNS 管理者が独自に追加したスタイルシートなど OpenPNE が関知していない CSS には適用できないため OpenPNE 側でこの問題は対応できない。

#10 Updated by Minoru Takai over 7 years ago

  • (C1) opSkinClassicPlugin の CSS に記述されているセレクタと、実際に出力される HTML 上の id 値が一致していない(3.6.x 以降)
    • これは具体的には HTML 上では <li id="globalNav__member_search"> のようにアンダースコアが 2 連続になってしまっている箇所があるが、 CSS 上ではこれと一致しない li#globalNav_member_search のように(アンダースコアは連続させずに)記述している箇所がある(note-9 に書いているが li#default__homepage など、 CSS 上でアンダースコアを連続している箇所もある)。
  • 関数定義とテンプレート記述
    function op_url_to_id($uri)
    {
      return str_replace(array('/', ',', ';', '~', '?', '@', '&', '=', '+', '$', '%', '#', '!', '(', ')'), '_', $uri);
    }
    
    <li id="globalNav_<?php echo op_url_to_id($nav->uri) ?>">
    
  • op_url_to_id() 関数の定義が適切でない件について、現時点の master ブランチのソースコードを用いた SNS において、グローバルナビ出力部分で array($nav->uri, op_url_to_id($nav->uri)) をデバッグ出力した結果を示しておきます。ここで出力されている id 値(array の [1] の方)に対応する、修正前の CSS は note-8 の修正差分 (変更前の記述)で確認できます。
    array
      0 => 
        array
          0 => string '@homepage' (length=9)
          1 => string '_homepage' (length=9)
      1 => 
        array
          0 => string '@member_search' (length=14)
          1 => string '_member_search' (length=14)
      2 => 
        array
          0 => string '@community_search' (length=17)
          1 => string '_community_search' (length=17)
      3 => 
        array
          0 => string '@ranking' (length=8)
          1 => string '_ranking' (length=8)
      4 => 
        array
          0 => string '@album_list' (length=11)
          1 => string '_album_list' (length=11)
      5 => 
        array
          0 => string 'diary/index' (length=11)
          1 => string 'diary_index' (length=11)
      6 => 
        array
          0 => string 'blog/index' (length=10)
          1 => string 'blog_index' (length=10)
      7 => 
        array
          0 => string '@member_config' (length=14)
          1 => string '_member_config' (length=14)
      8 => 
        array
          0 => string '@member_invite' (length=14)
          1 => string '_member_invite' (length=14)
      9 => 
        array
          0 => string '@member_logout' (length=14)
          1 => string '_member_logout' (length=14)
    

#11 Updated by Minoru Takai over 7 years ago

  • Description updated (diff)

#12 Updated by Kousuke Ebihara over 7 years ago

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

#1004 にて、 fixture でインポートされる各ナビゲーション項目の URL が、内部 URL (デフォルトルートにマッチさせる形式)からルート名を指定したものに置き換わっていますが、既にこの fixture をインポート済みのデータ(要は OpenPNE 3.4 時点で OpenPNE 2 からのアップグレードや新規インストールを完了させた状態のデータ)に格納された URL は置き換わりません(ナビゲーションは出力がキャッシュされるので、従来の表記を放置したところでパフォーマンス上の影響は小さく抑えられるため、このこと自体は特に大きな問題はないと思っています)。

このチケットでおこなわれた 582fc718 の変更は前述のような状態のデータを考慮していません。わかりやすく言うと、 3.4 時点で OpenPNE 2 からのアップグレードをおこなった SNS がこの変更を適用後の OpenPNE にアップデートした場合、このチケットで報告されているものと同じ(ような)問題に遭遇することになると思います。

ですので、 582fc718 の対応方針を採るのであれば、 OpenPNE 3.6 時点の fixture によって作られるデータだけではなく、 3.4 時点の fixture によって作られるデータも考慮するべきです。

また、他の対応方針として、 (S2) の対応方法の一例として挙げられている op_url_to_id() の実装を変更するのも手です。ただ、 op_url_to_id() は opSkinClassicPlugin に限定して使われているわけではなく、このヘルパー関数の実装の変更は他のスキンプラグインやカスタム CSS などに影響する恐れがあります。従って、 op_url_to_id() を変更する場合、このチケットの対応に必要な最低限度の変更にとどめる(どうやって?)など、できるだけ影響を小さくするように配慮することが求められます。

以上、検討をお願いします。

#13 Updated by Minoru Takai over 7 years ago

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

note-12 の指摘について、 582fc718 の修正が 3.4.x 以前の fixture によって作られるデータを考慮できていない点は、このコミット時点で気づいていませんでした(というより、この問題は 3.4.x にも共通する問題だと思っていました)。コミット後に 3.4.x で状況が違うことに気付き、note-7 の内容を調査をしました。

note-12 と note-7 以降の調査結果を踏まえて、改めて修正を行います。

582fc718 は (C1) と (C2) を同時に扱ってしまっており差分の意味が分かりにくいので、一旦これを revert しようと思います。


note-12 で、

従って、 op_url_to_id() を変更する場合、このチケットの対応に必要な最低限度の変更にとどめる(どうやって?)など、できるだけ影響を小さくするように配慮することが求められます。

と言われている通り、他所でこの関数(およびその結果)を使っているところへの影響が無いようにする必要がありますが、 note-9 で、

  • (S2): HTML 側の出力(恐らく op_url_to_id() 関数の定義)を修正する(関数定義は変えないで、テンプレート側で、「先頭に _ が付いていたら除去する」といった対応も可能だろうか)

と記述しているように、「ナビゲーションを表示するテンプレート側で先頭に _ が付いていたら除去する」修正がよいのではないかと考えています。

現時点で思いついている修正案を示します。

  • 修正前
    function op_url_to_id($uri)
    {
      return str_replace(array('/', ',', ';', '~', '?', '@', '&', '=', '+', '$', '%', '#', '!', '(', ')'), '_', $uri);
    }
    
    <li id="globalNav_<?php echo op_url_to_id($nav->uri) ?>">
    
  • 修正案(1) : 互換性を保ったまま関数定義を変える(ナビのために ltrim() だけ追加するのは不自然なので trim() にしています)
    function op_url_to_id($uri, $isTrim = false)
    {
      $str = str_replace(array('/', ',', ';', '~', '?', '@', '&', '=', '+', '$', '%', '#', '!', '(', ')'), '_', $uri);
      if ($isTrim)
      {
        $str = trim($str, '_');
      }
    
      return $str;
    }
    
    <li id="globalNav_<?php echo op_url_to_id($nav->uri, true) ?>">
    
  • 修正案(2) : テンプレート側で文字列処理をする
    function op_url_to_id($uri)
    {
      return str_replace(array('/', ',', ';', '~', '?', '@', '&', '=', '+', '$', '%', '#', '!', '(', ')'), '_', $uri);
    }
    
    <li id="globalNav_<?php echo ltrim(op_url_to_id($nav->uri), '_') ?>">
    

#14 Updated by Minoru Takai over 7 years ago

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

更新履歴 8cd31b77d55b255f764b898ed4d47989ae8113b9 で適用されました。

#15 Updated by Minoru Takai over 7 years ago

更新履歴 05a1c7fad7b30481da7d998d8c0cce4294a17af2 で適用されました。

#16 Updated by Kousuke Ebihara over 7 years ago

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

#17 Updated by Minoru Takai over 7 years ago

note-9 で次のように示していますが、

  • (S2): HTML 側の出力(恐らく op_url_to_id() 関数の定義)を修正する(関数定義は変えないで、テンプレート側で、「先頭に _ が付いていたら除去する」といった対応も可能だろうか)
    • 【必要な修正】 3.4.x 以前( #257 時点)から、もともと @homepage のようにナビゲーション設定で URL を記述していた部分は、CSS上でも li#default__homepage と記述しているようなので、もともと CSS にアンダースコアを連続して記述していた部分を書き換える必要がある。
    • 【懸念点】 これまでにカスタムCSSなどを記述しているとセレクタがマッチしなくなるためにレイアウトが崩れたりする SNS が現れる可能性がある。これは SNS 管理者などに対応してもらう必要があるため、この問題についてブログ告知するなりして周知する必要がある。
      • li 要素の id 属性値に default__homepage と default_homepage の両方を指定しておくといったことで対応するのは本質的ではないし(特に id 属性なので文法違反であり対応不可能)、機械的に CSS 側のセレクタを解釈し直すことも現実的ではなく、特に SNS 管理者が独自に追加したスタイルシートなど OpenPNE が関知していない CSS には適用できないため OpenPNE 側でこの問題は対応できない。

今回、この (S2) の対応を行いました。

この修正が入る前の OpenPNE で HTML に出力されていたナビの id 属性値に対する CSS (つまり li#globalNav__homepage のようなセレクタ)をカスタムCSSなどで記述している場合、この修正によってナビの id 属性値が __* のものは _* へと変わるためにスタイル指定が無効になってしまう可能性があります。この問題について、この修正が取り込まれる最初のリリース OpenPNE-3.6.0 において、リリース告知記事内でこのことを周知したいと思います。

#18 Updated by isao sano over 7 years ago

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

テスト完了致しました。
問題は発見されなかったため、ステータスをFixed(完了)に致します。

#19 Updated by kaoru n over 3 years ago

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

Also available in: Atom PDF