サイト内検索
Cocoonフォーラム
![](https://wp-cocoon.com/wp-content/themes/cocoon-master/screenshot.jpg)
書き込みの前に以下の3点をご確認ください。
何を書き込んだら良いか分からない場合は、以下のテンプレートをコピペしてご利用ください。
不具合・カスタマイズ対象ページのURL:
相談内容:
不具合の発生手順:
解決のために試したこと:
※文字だけでは正しく伝わらない可能性があるため、スクショ画像の添付もお願いします。
※高速化設定をしている場合は無効にしてください。
環境情報:※↑こちらに「Cocoon設定 → テーマ情報」にある「環境情報」を貼り付けてください。
環境情報の取得方法はこちら。
→ https://wp-cocoon.com/theme-report/
高速化設定を無効にするにはこちら。
→ https://wp-cocoon.com/theme-trouble/
フォーラム利用ガイドリンク
- フォーラムガイドライン
- よくある質問と答え(FAQ)
- サポート対象外のケース
- 原因不明の不具合用トラブルシューティング
- トピックにHTMLを貼り付ける方法(推奨ツール:notepad.pw)
- 真っ白画面でのエラーメッセージの確認方法
- ブラウザ環境チェックツール
- Cocoonカスタマイズ依頼
フォーラム質問後、問題等が解決した場合は結果を書き込んでいただけると幸いです。同様の問題で調べている方には、結果が一番気になる部分となります。
Topic starter
2019年8月13日 23:34
「Cocoon設定」で用意されている各種設定をグローバル変数$_THEME_OPTIONSで変更できる機能があります。
このグローバル変数$_THEME_OPTIONSに関して、不具合と要望があります。
回避する方法は一応あるので、回避用のコードを追加しつつ対応してきたものの、カスタマイズの広範囲に渡って影響してきたので、できればテーマ側で解決していただきたいと思い報告させていただきます。
【不具合】
カスタマイズでCocoon設定の値を取り扱う際、保存されたデータに加えて$_THEME_OPTIONSを考慮する必要があります。
lib/page-settings/○○-funcs.phpに書かれた各種設定値の取得関数(つまり、get_theme_option関数を用いて値を取得している関数)を用いるのが手っ取り早いのですが、これらの関数を用いるとget_theme_option関数内で$_THEME_OPTIONSから値を取得する際の判定
if ($skin_option !== null && !is_admin_php_page()) {
https://github.com/yhira/cocoon/blob/master/lib/utils.php#L256
によって、「Cocoon設定」ページのみ不具合が生じる場合があります。
上記の説明だけでは分かりづらいかもしれませんので、Cocoon本体内の挙動をもとに1例挙げておきます。
<例>
Cocoonのスキンを使用する場合、get_skin_url関数(キー名'skin_url')で取得する設定値から、テーマカスタマイズ用の各スキンファイルが読み込まれます。
そしてスキンはスタイルだけでなく、PHP等のカスタマイズも加えられるようになっています。
グローバル変数を用いて、
$_THEME_OPTIONS['skin_url'] = 'https://wp-cocoon.com/wp-content/themes/cocoon-master/skins/skin-fuwari-kachiiro/style.css'
と値を変更したとします。
一方、キー名'skin_url'としてデータベースに保存されている値は'https://wp-cocoon.com/wp-content/themes/cocoon-master/skins/skin-season-summer/style.css'となっているとしましょう。
すると、サイト全体にはFuwari(褐色)のスキンファイルが反映されますが、設定画面ではSeason(Summer)のスキンファイルが反映されてしまいます。
例のようにget_theme_option関数を用いた設定値の取得方法だと、Cocoon設定ページでは値が正しく反映されないことにより管理画面内に不具合が生じるので、カスタマイズではCocoon設定の各関数を流用することができず、別途値を取得するコードを書かないといけなかったり、設定ページのみ異なる挙動になってしまう場合があります。
これを改善するためには、is_admin_php_page関数でページごと$_THEME_OPTIONSからの値を取得不可にしてしまうのではなく、フォーム内フィールドに入れる値かどうかで分ける必要があります。
【要望】
$_THEME_OPTIONSを用いてスキンから設定値を変更する方法として、
- PHP
- CSV
- JSON
の3通り用意されています。
スキンによる設定値の変更は
https://github.com/yhira/cocoon/blob/master/lib/skin.php
にて$_THEME_OPTIONSにセットされています。
この中でPHPで$_THEME_OPTIONSにセットする場合、
https://wp-cocoon.com/skin-functions-php/
の方法が書かれてあり、実際にいくつかのスキンではこの形で書かれています。
$_THEME_OPTIONSの配列をスキンのfunctions.phpで新たに代入すると、スキンのfunctions.php実行以前では$_THEME_OPTIONSを使用することができず、例えば「デフォルトの変更値→スキンの変更値」のようなスキンを優先する変更値の設定ができないので、スキンのfunctions.php実行後に値をキーごとに判定しつつ、適宜代入するように書く必要が出てきます。
CSVやJSONのように、
$_THEME_OPTIONS[$name] = $value;
キーごとに変更値を設定するようにし、スキンのfunctions.php実行以前でも$_THEME_OPTIONSを使用可能な形にすることはできないでしょうか?
2019年8月14日 18:14
lib/skin.phpは、lib/page-settings/skin-funcs.phpの後に読み込んでいるので、スキン設定項目に関しては、値は反映されないかもしれません。
https://github.com/yhira/cocoon/blob/edf9a3d874e1febe4fe6a2e2c6289c75c5516922/lib/_imports.php#L15
ここら辺は、自分でもよくないとは思いつつ、改善方法は思いついていません。
※スキン用の設定なんて、スキン制御をかけたい人も少ないだろうという甘えから後回しにしてました。
これを改善するためには、is_admin_php_page関数でページごと$_THEME_OPTIONSからの値を取得不可にしてしまうのではなく、フォーム内フィールドに入れる値かどうかで分ける必要があります。
すいません。これは、どのようなことをするのか、ちょっとわからなかったです。
キーごとに変更値を設定するようにし、スキンのfunctions.php実行以前でも$_THEME_OPTIONSを使用可能な形にすることはできないでしょうか?
こちらのように読み込んでしまうと、$_THEME_OPTIONS変数自体の中身が、「スキンのfunctions.php」に書かれたものに入れ替わってしまうということですよね。
https://wp-cocoon.com/skin-functions-php/
なので、CSVや、JSONのように新しい屋台があるときだけ、配列に対して新しい値追加していくということをですよね。
今のところ、親テーマで使われている、グローバル変数$_THEME_OPTIONSを全て別の名前に変更(例えば$_COCOON_THEME_OPTIONSに)して、こちらの処理部分を変換して
https://github.com/yhira/cocoon/blob/master/lib/skin.php#L12-L16
以下のように追加していくことぐらいしか思いついていないです。
$_COCOON_THEME_OPTIONS[$name] = $value;
こうすれば、「スキンのfunctions.php」の記述を変更する必要がなくなるので良いのかなと。
他に、何か良い方法とかをご存知でしょうか。
Topic starter
2019年8月14日 20:39
まずは不具合の方についてです。
lib/skin.phpは、lib/page-settings/skin-funcs.phpの後に読み込んでいるので、スキン設定項目に関しては、値は反映されないかもしれません。
ファイルの読み込み順はあまり関係ないと思います。
lib/skin.phpでグローバル変数$_THEME_OPTIONSに値がセットされるのはテーマファイル読み込み時である一方、lib/page-settings/skin-funcs.php等の関数が実行されるのは管理画面内で設定ページを開く時だからです。
問題となっているのは最初の投稿に示したように、get_theme_option関数で$_THEME_OPTIONSから変更値を取得する際、is_admin_php_page関数によってCocoon設定ページのみでは全て無効となり、データベースに保存された値を取得します。
つまりCocoon設定ページでは、ベースにget_theme_option関数が用いられるプログラム全てについて、本来の動作とは異なる動作が実行されうるということです。
設定ページで一番最初の設定が'skin_url'なのでスキンを例に挙げましたが、飽くまで一例で他の設定項目についても同様のことが言えます。
「スキンのスタイルを固定するためにヘッダーの設定で'header_layout_type'を'top_menu'、'tagline_position'を'none'になるよう$_THEME_OPTIONSに変更値を設定しても、別の値が保存されていればプレビュー画面には変更値が反映されず、プレビュー上でのデザインが崩れる」といった例も考えられます。
get_theme_option関数が実行される機能やカスタマイズでは、$_THEME_OPTIONSとの関係から本来表示されるべき管理画面内の設定がCocoon設定ページでは表示されなかったり、実行されるべきでないプログラムがプレビュー画面上で実行されたり…などの問題を考慮しなくてはいけません。
よって、この問題がある限りはget_theme_option関数の実行を避ける形でコードを書く必要が出てきます。
(逆に避けないのであれば、$_THEME_OPTIONSの機能を用いるべきでないということになってしまいます。)
では、なぜget_theme_option関数内でis_admin_php_page関数が用いられているかと言うと、コードを読んだ限りでは「Cocoon設定ページのフォームにて設定した値を$_THEME_OPTIONSの変更値に上書きしないため」と理解しています。
これを改善するためには、is_admin_php_page関数でページごと$_THEME_OPTIONSからの値を取得不可にしてしまうのではなく、フォーム内フィールドに入れる値かどうかで分ける必要があります。
以上のことから、「Cocoon設定ページでも各プログラムが正しい挙動で実行されつつ、設定値が変更値に上書きされないようにする」ためには、「ページ全体で$_THEME_OPTIONSの取得を無効にしてしまうのではなく、フォーム内の各フィールドに値を入れる場合とそうでない場合で分ける方法に修正する必要がある」と考えました。
続いて、要望の方についてです。
こちらのように読み込んでしまうと、$_THEME_OPTIONS変数自体の中身が、「スキンのfunctions.php」に書かれたものに入れ替わってしまうということですよね。
そういうことです。
スキンのみを対象にした機能であればグローバル変数を用いた機能にする必要はなく、グローバル変数を用いた機能であれば推奨される方法のようなスキンでの使用を前提にしてしまうより、スキンファイルの実行前後でも使用できることを前提にした方が好ましいと言いますか、グローバル変数である価値は高いと思います。
他に、何か良い方法とかをご存知でしょうか。
スキンで一度使用されるだけなのであれば、わざわざ新たにグローバル変数を用意する必要はないと思います。
関数で囲ってそのまま配列を返す形にし、lib/skin.phpで$_THEME_OPTIONSの配列に結合するなどでよいのではないでしょうか。
2019年8月14日 22:05
ファイルの読み込み順はあまり関係ないと思います。
僕の記憶では、なんか不具合があったので、読み込み順は現在のようになっているのですが、今回の件とは関係なかったんですね。
では、なぜget_theme_option関数内でis_admin_php_page関数が用いられているかと言うと、コードを読んだ限りでは「Cocoon設定ページのフォームにて設定した値を$_THEME_OPTIONSの変更値に上書きしないため」と理解しています。
これはその通りです。
設定保存時にスキン設定値で、Cocoon設定値を上書きしないためです。
それは踏まえている上で、申しわけないのですが、以下の部分がいまいちピンときていません。
「ページ全体で$_THEME_OPTIONSの取得を無効にしてしまうのではなく、フォーム内の各フィールドに値を入れる場合とそうでない場合で分ける方法に修正する必要がある」
保存時に$_THEME_OPTIONSの値は判別して、不要なものは保存しないようにするということでよいのでしょうか。
以下のような回避方法を追加されたのであれば、よろしければ一部でも良いので、コードを見せていただけると幸いです。
回避する方法は一応あるので、回避用のコードを追加しつつ対応してきたものの、カスタマイズの広範囲に渡って影響してきたので、できればテーマ側で解決していただきたいと思い報告させていただきます。
書き込みの意味を、うまく理解できておらず申しわけないです。
2019年8月14日 22:14
関数で囲ってそのまま配列を返す形にし、lib/skin.phpで$_THEME_OPTIONSの配列に結合するなどでよいのではないでしょうか。
これは、スキンのfunctions.php側でのことですか?
2019年8月14日 22:27
というか、親テーマ側のこの部分で、$_THEME_OPTIONSをテンポラリー(一時的な)変数にでも入れといて、
https://github.com/yhira/cocoon/blob/edf9a3d874e1febe4fe6a2e2c6289c75c5516922/lib/skin.php#L10
そのテンポラリー変数と、こちらで上書きされた$_THEME_OPTIONSをマージして新たに、$_THEME_OPTIONSに再び代入するとかでもいいのかな。
https://github.com/yhira/cocoon/blob/edf9a3d874e1febe4fe6a2e2c6289c75c5516922/lib/skin.php#L15
もし現在、対応されている方法もしくは、希望の方法がありましたら、その方法にするので、さわりだけで良いのでnotepad.pwにでも貼っていただけると助かります。
どうしても、文字だけだと認識の齟齬が出てしまい、僕がロコさんの意図していない解釈をしてしまう可能性があるので^^;
2019年8月14日 22:34
とりあえず、前回の返信も、lib/skin.php冒頭の流れだけ簡単にだけ書くとこんな感じ。
global $_THEME_OPTIONS;
$temp_options = $_THEME_OPTIONS;
//スキン用のfunctions.phpがある場合
$php_file_path = url_to_local(get_skin_php_url());
if (file_exists($php_file_path)) {
require_once $php_file_path;
}
$_THEME_OPTIONS = array_merge($temp_options, $_THEME_OPTIONS);
※スキンのfunctions.phpで$_THEME_OPTIONSが指定されているのを前提にしたコードなので、後でちゃんと書きます。
Topic starter
2019年8月14日 23:25
- $_THEME_OPTIONSの機能がある以上はCocoon設定の全ての値が$_THEME_OPTIONSによって変更できるのを考慮する必要がある。
- Cocoon設定ページを開く際、フォーム内の各フィールドでvalueとして入れるとき以外でもget_theme_option関数が実行される(管理画面全般やプレビュー画面として取得するテンプレート内など)。
- get_theme_option関数で取得する値の判定をis_admin_php_page関数でしてしまうと、ページ内全域に渡って$_THEME_OPTIONSが反映されない。
- 特に$_THEME_OPTIONSによって設定を制限し、get_theme_option関数が実行されるプログラムを用いてそのままカスタマイズしてしまうと、Cocoon設定ページではカスタマイズ部分が反映されなかったり、意図した値とは異なる値で動作するなど、不具合が生じてしまう場合が出てくる。
- Cocoon設定ページでも正しく動作させるには、get_theme_option関数の実行を避ける、全ての値で同様に実行するといった対策を講じる必要がある。
まず、ここまではよろしいでしょうか?
この問題を解決するためには、get_theme_option関数内でis_admin_php_page関数による判定をどうにかしないといけません。
解決できるのであればどのような方法でも構いませんが、私はget_theme_option関数からis_admin_php_page関数による判定をなくすのが手っ取り早いのかなと思いました。
すると、
Cocoon設定ページのフォームにて設定した値を$_THEME_OPTIONSの変更値に上書きしない
という問題が出てきますが、これについてはフォーム内の各フィールドに入れる値はget_theme_modでそのまま取得するようにでもしておけば解決できます。
get_theme_option関数を使用するのであれば、フィールドに入れる場合→false、それ以外→trueと受け渡して判定するなども考えられます。
修正案をコードにしておきます。
問題が解決できるのであれば、別の修正方法でも構いません。
generate_radiobox_tag(OP_SKIN_URL, $options, get_theme_mod(OP_SKIN_URL, ''));
https://github.com/yhira/cocoon/blob/master/lib/page-settings/skin-forms.php#L82
のように、各フィールドに値を入れる際はデータベースの値をそのまま用います。
if ($skin_option !== null) {
https://github.com/yhira/cocoon/blob/master/lib/utils.php#L256
すると、get_theme_option関数内で$_THEME_OPTIONSの変更値取得を無効にする必要がなくなるので、Cocoon設定ページでもget_theme_option関数が実行されるプログラムが正しく動作するようになります。
わいひら reacted
Topic starter
2019年8月14日 23:39
そのテンポラリー変数と、こちらで上書きされた$_THEME_OPTIONSをマージして新たに、$_THEME_OPTIONSに再び代入するとかでもいいのかな。
この方法でも問題はないかと思います。
これは、スキンのfunctions.php側でのことですか?
ここで説明したのは、スキン側では
function get_skin_theme_options() {
return array(
...
);
}
そして、lib/skin.phpでは該当の関数が存在する場合に
$_THEME_OPTIONS = array_merge($_THEME_OPTIONS, get_skin_theme_options());
といった形でした。
わいひら reacted
2019年8月15日 22:38
まず、ここまではよろしいでしょうか?
それは存じております。
これまであえてis_admin_php_pageでブロックさせていたので。
※これまでは、それが実装時手っ取り早かったのと、スキン制御値が保存されるよりも良いかと思っていたのと、Cocoon設定に$_THEME_OPTIONS自体を反映表示させる必要がそこまであるか、と思っていたことがあります。
ただ、スキン制御がプレビューなどに反映されないのは、確かに気持ちの悪い部分であるのは間違いないと思います。
この機会に修正してしまおうとは思います。
generate_radiobox_tag(OP_SKIN_URL, $options, get_theme_mod(OP_SKIN_URL, ''));
とのことなので、設定項目値(フィールド値)自体に、スキン制御値($_THEME_OPTIONS)が反映されないのはいいんですね。
以下のようなプレビュー、テンプレート部分に.スキン制御の値が反映されていれば良いということですよね?
(管理画面全般やプレビュー画面として取得するテンプレート内など)
僕は、全てのCocoon設定フィールドに対しても、値を反映させるのだと思っていました。
なので、どのように?と思ったので質問させていただきました。
ただ、フィールド値に入れる値をget_theme_modにしたり、true/falseを持たせる方法も、結構を書き換える必要があるので、出来る限り楽できる方法を模索してみようと思います。
無理そうなら、どちらかの方法にします。
2019年8月15日 22:39
ここで説明したのは、スキン側では
これだと、これまでの仕様が変わってしまうので、テンポラリー変数を使う方法にとりあえずしたいと思います。
しかし、スキンのfunctions.php側では、get_skin_theme_options()関数などを使った方が望ましいかなと思うので。どちらでも利用できるようにしておこうと思います。
https://github.com/yhira/cocoon/commit/f5a9bc6d6147dee16e34cd3520a15bc236a4d502
後で忘れなければ、ドキュメントにも関数での指定方法も書いておこうと思います。
Topic starter
2019年8月15日 23:58
Cocoon設定に$_THEME_OPTIONS自体を反映表示させる必要がそこまであるか、と思っていたことがあります。
ここでは分かりやすいようにスタイル等が反映されない例で説明していますが、実際に$_THEME_OPTIONSで値を制御しつつ機能を実装してみるともっと大きな問題が出てきます。
WordPress本体で言うと、特定ページの$postに別ページのものが入っているのをイメージするのが分かりやすいのかもしれません。
設定項目値(フィールド値)自体に、スキン制御値($_THEME_OPTIONS)が反映されないのはいいんですね。
データベースに保存されている値はそのままで大丈夫です。
$_THEME_OPTIONSの値に変更すべきではないと私も思います。
保存される値がそのままでよい理由としては、Cocoon設定が関わる機能はベースにget_theme_option関数が用いてられており、get_theme_option関数が$_THEME_OPTIONSを優先した上で正しい値を返せばよいからです。
($_THEME_OPTIONSに値があればデータベースに保存された値が何であろうと$_THEME_OPTIONSの値を返し、$_THEME_OPTIONSに値がなければそのままデータベースに保存された値を返す。)
以下のようなプレビュー、テンプレート部分に.スキン制御の値が反映されていれば良いということですよね?
データベースへの保存・更新に用いられる値とそれ以外で用いられる値を切り分けて、データベースへの保存・更新に用いられる値以外ではget_theme_option関数が正しい値を返すようになればよいということです。
他に何か分からない点があれば、また説明を加えます。
修正よろしくお願いします。
わいひら reacted
Topic starter
2019年8月16日 01:33
$_THEME_OPTIONSのスキン前後での使用とget_skin_theme_options()関数については確認しました。
修正ありがとうございます。
わいひら reacted
2019年8月16日 23:22
どうしても文章だけだと、いろいろな意味にとれてしまう部分があるので、なかなか理解できず申しわけないです。
先日コードを少しいただけたおかげで、意図は多分理解できたと思います。おそらく…。
-----------
修正については、せっかく全ての値がget_theme_optionを通っているので、出来ればここら辺だけで解決したいところではあります。今後のメンテナンス的にも。
ちょっと今日は、これに関しては何もできなかったので、明日あたりブランチを切ってそこにコードを書いてみたいと思います。
$_THEME_OPTIONSのスキン前後での使用とget_skin_theme_options()関数については確認しました。
こちらご確認ありがとうございます。
Topic starter
2019年8月17日 03:03
修正点の確認として、再度コードを交えながらまとめておきます。
if ($skin_option !== null && !is_admin_php_page()) {
https://github.com/yhira/cocoon/blob/master/lib/utils.php#L228
問題なのは上記コードの赤字で示した判定部分です。
generate_form_tag($name = 'cocoon_name', $value = get_theme_option('cocoon_name', $default), ...);
(generate_form_tagはgenerate_selectbox_tag、generate_checkbox_tag、generate_textbox_tagなどのフィールドを生成する関数とする。)
Cocoon設定の各フィールドは、間にある関数の経由を除くと実質上記のような形で表せると思います。
function get_theme_option($option_name, $default = null){
$skin_option = get_skin_option($option_name);
if ($skin_option !==null) {
if ($skin_option =='0') {
$skin_option = 0;
}
return $skin_option;
}
return get_theme_mod($option_name, $default);
}
判定がない状態だと、
generate_form_tag($name = 'cocoon_name', $value = $_THEME_OPTIONS['cocoon_name'], ...);
となる場合があり、$_THEME_OPTIONSによって保存データを書き換えられてしまうことになります。
generate_form_tag($name = 'cocoon_name', $value = get_theme_mod('cocoon_name', $default), ...);
(get_theme_option関数を使用する場合)$_THEME_OPTIONSによる書き換えを防ぐために、Cocoon設定内の各フィールドに入れる値のみget_theme_option関数がget_theme_mod関数と同値となるよう、必要十分条件で
https://github.com/yhira/cocoon/blob/master/lib/utils.php#L228
の判定を記述する必要があります。
フィールド外までget_theme_option関数や$_THEME_OPTIONSに条件を持ち込むと、カスタマイズを加えた制作者やWordPressユーザーの意図とは異なる動作が実行される($_THEME_OPTIONSでの制御が効かない)可能性を残してしまうからです。
フィールド以外にもCocoon側で$_THEME_OPTIONSの値を返したくない明確な箇所があれば別ですが、ひとまずは以上のような感じです。
2019年8月17日 23:23
フィールドに対してget_theme_modを「ヘッダーレイアウト」にだけ適用させたものは書いてみました。
こういうことですよね?
https://github.com/yhira/cocoon/commit/6237950f1db947621bab3d0903ac5c5d81753ac7
ただ、これだと全てのフィールドを書き換える必要があるので、何とか、楽できる方法はないものか、いろいろ試してはいます。
けれど、今のところ、他に良い方法は見つからず…。
Topic starter
2019年8月18日 00:35
そういうことです。
generate_selectbox_tag(OP_HEADER_LAYOUT_TYPE, $options, get_theme_mod(OP_HEADER_LAYOUT_TYPE, 'center_logo'));
こちらのような形での実装をCocoon設定にある全てのフィールドに対して行う代わりに、
if ($skin_option !== null && !is_admin_php_page()) {
この条件が実装されたと思うのですが、is_admin_php_page関数の条件が広すぎるのが今回の問題なので、この条件を本来求めていた条件と1対1になるよう修正するのが不具合のない確実な修正方法ということです。
2019年8月18日 19:43
こちらの方法だと、どうしても修正負担が高いのと、新たにDBデータ取得用の関数でも書かない限り(もしくは引数で条件分岐とかしない限り)、get_theme_mod(OP_HEADER_LAYOUT_TYPE, 'center_logo')コードが2ヶ所重複してしまうので別方法でアプローチしてみることにしました。
https://github.com/yhira/cocoon/commit/6237950f1db947621bab3d0903ac5c5d81753ac7
新たな方法としては、とりあえず、Cocoon設定フィールドに「スキン制御値」は取り込むけど、set_theme_modで保存時に、「スキン制御値」は保存しないという方法にしてみました。
https://github.com/yhira/cocoon/commit/44762827148a78eeceed3cc2c9f4d99edc68022b
https://github.com/yhira/cocoon/tree/skin_post_block
これだと、Cocoon設定上でも「スキン制御値」が添付画像のように表示されるので、現在の状態を把握しやすくなるかと思います。もちろん、正しく動作していればですが(動作確認はしています)。
Topic starter
2019年8月18日 20:38
修正ありがとうございます。
ブランチの「skin_post_block」をダウンロードして軽くテストしてみたのですが、正しく動作していない点が出てきました。
修正後のコードを読みつつ、再度問題点を洗い出そうと思いますが、少し時間がかかるかもしれません。
とりあえず報告までに。
【追記】
まだテストを始めたばかりで詳しくは見ていませんが、今確認できた問題点としては保存後の画面で$_THEME_OPTIONSが正しくありません。
わいひら reacted
Topic starter
2019年8月18日 21:30
追記に関する補足を加えておきます。
get_theme_option関数からis_admin_php_page関数による判定がなくなったことによって、管理画面上でざっと見た限りでは最初にCocoon設定ページを開いた際には$_THEME_OPTIONSの制御が一通り(管理画面内の設定やプレビュー画面のスタイル、個別フィールドのスキン制御用ラップタグなど)反映されているようですが、設定を保存した後の設定画面上で反映されない箇所が出てきました。
これが設定値が保存されるタイミングとの兼ね合いなのか、他の影響なのかは調査中です。
わいひら reacted
2019年8月18日 21:55
ご確認ありがとうございます。
一応、いろいろな動作確認はしていたのですが、不具合がありましたか…。
「使用されているスキン制御」と「どの設定項目に不具合が出ているか」を書いていただければ、こちら側でもテストしてみます。
Topic starter
2019年8月18日 22:31
設定値の保存が正しく行われているかどうかについてはまだ確認していませんが、それ以外のところで生じる1点の不具合が見つかりました。
https://github.com/yhira/cocoon/blob/skin_post_block/lib/page-settings/_top-page.php#L112-L113
で$_THEME_OPTIONSが一旦リセットされることにより、本来反映されるべき$_THEME_OPTIONSの制御が全て無効になってしまうのが問題のようです。
今回の修正だと少なくとも、何らかの方法で設定変更前の$_THEME_OPTIONSから旧スキン制御部分のみリセットする必要があると思います。
この点に関しては、修正前のように「_imports.phpでは読み込まず、設定値が変更されてから読み込む」という形が綺麗だったのですが、そうするとupdate_theme_option関数の制御が正しくできず…。
プログラム上での$_THEME_OPTIONS制御による不具合の影響としては、
- 修正前→is_admin_php_page関数がグローバル変数$pagenowでCocoon設定ページと判定できる時点から、全プログラムの実行が完了するまで。
- 修正後→最初のCocoon設定ページ読み込み(page-settings/_top-page.php)から、保存処理実行のために再度ページ読み込みが始まり、保存処理が完了するまで。
ということになりそうです。
Topic starter
2019年8月19日 00:41
先ほどの書き込みに補足と追記です。
修正後→最初のCocoon設定ページ読み込み(page-settings/_top-page.php)から、保存処理実行のために再度ページ読み込みが始まり、保存処理が完了するまで。
厳密には「$_THEME_OPTIONS制御への影響」で、$_THEME_OPTIONS制御を用いたプログラムの実行まで含めると影響の範囲は上記より広くなります。
少なくとも、何らかの方法で設定変更前の$_THEME_OPTIONSから旧スキン制御部分のみリセットする必要がある
そして、具体的なテストはまだですが、上記のように修正したと仮定すると、加えて最初の読み込み~保存処理実行間の$_THEME_OPTIONS制御と保存処理を切り離す必要がありそうです。
「最初のCocoon設定ページ読み込み時にget_theme_option関数内で用いられる$_THEME_OPTIONSと、保存処理の実行で再度ページ読み込みが行われる際にupdate_theme_option関数内で用いられる$_THEME_OPTIONSを一致させるような処理を加える」ということです。
(例えば、最初の読み込み時に用いる$_THEME_OPTIONSを変更できない形で保持して、その保持したものを保存処理実行時の$_THEME_OPTIONSとして用いるなど。)
同じページではありますが、保存処理を行うためにページの再構築を挟むということは、$_THEME_OPTIONS制御もまた0から生成が行われるということになります。
修正前の形であれば、そのまま保存後の設定ページとして新たな設定を行うことができますが、修正後の形だとそのままでは保存処理に影響してしまうことになるかと思います。
「ページ変移なしで先にフォームのデータの保存処理を実行→保存処理完了後にページを再読み込み」
のような実行順序であれば大きな問題はなさそうなのですが、そのような処理を書いていると結局一番最初に示した修正が楽になってしまいそうなので…。
2019年8月19日 00:52
よろしければ、具体的な不具合の再現方法書いていただいてよろしいでしょうか。
例えばこんな感じで。
- ○○というスキンの選択して保存
- ××のスキンを選択して保存
- すると、□□の値がおかしい(もしくは他の症状、どこどこのプレビューがおかしいとか)
僕の環境だと、ちょっと不具合自体が再現できていません。
上で書いた手順みたいな再現方法かわかりませんが、不具合の再現方法がわかれば、おそらく対応できるのではないかと思います。
Topic starter
2019年8月19日 01:16
私の環境だと$_THEME_OPTIONSは動的制御が多く複雑なので、分かりやすく静的制御で例を示します。
global $_THEME_OPTIONS;
$_THEME_OPTIONS['amp_enable'] = 0;
例えば、上記を子テーマfunctions.phpにでも記述するとしましょう。
最初のCocoon設定ページ読み込み時には反映されるものの、
https://github.com/yhira/cocoon/blob/skin_post_block/lib/page-settings/_top-page.php#L112-L113
リンク先のコードによって$_THEME_OPTIONSそのものがリセットされるため、保存実行後の設定ページでは反映されない、ということです。
つまり、修正の第一段階としては、必要な$_THEME_OPTIONS制御をリセットさせないよう修正する必要があるということになります。
Topic starter
2019年8月19日 01:44
修正の第二段階については主に動的制御が当てはまりますが、こちらも分かりやすく静的制御で説明を加えておきます。
$_THEME_OPTIONSが動的制御になると、最初のCocoon設定ページ読み込み時の$_THEME_OPTIONS([※1])と再度ページが読み込まれる保存実行時の$_THEME_OPTIONS([※2])が異なってくる場合があります。
特に、[※1]に入っている変更値の保存は防ぐべきなので、[※2]は[※1]に一致させないといけないということなのですが、例えば以下のような場合に保存処理が実行されてしまいます。
global $_THEME_OPTIONS;
$_THEME_OPTIONS['amp_enable'] = 0;
$_THEME_OPTIONS['pwa_enable'] = 0;
[※1]では上記の制御がセットされているとしましょう。
そして、元々データとして保存されていた'amp_enable'と'pwa_enable'の値は'1'とします。
[※1]によって、POSTされる'amp_enable'と'pwa_enable'の値は'0'となります。
global $_THEME_OPTIONS;
$_THEME_OPTIONS['amp_enable'] = 0;
[※2]では上記の制御がセットされた場合、本来保存処理が行われるべきでない'pwa_enable'が
リンク先の判定を通過します。
具体例はこのような感じでよろしいでしょうか?
修正よろしくお願いします。
2019年8月19日 20:34
とりあえず一つずつ修正していきます。
私の環境だと$_THEME_OPTIONSは動的制御が多く複雑なので、分かりやすく静的制御で例を示します。
global $_THEME_OPTIONS;
$_THEME_OPTIONS['amp_enable'] = 0;例えば、上記を子テーマfunctions.phpにでも記述するとしましょう。
最初のCocoon設定ページ読み込み時には反映されるものの、
https://github.com/yhira/cocoon/blob/skin_post_block/lib/page-settings/_top-page.php#L112-L113
リンク先のコードによって$_THEME_OPTIONSそのものがリセットされるため、保存実行後の設定ページでは反映されない、ということです。
つまり、修正の第一段階としては、必要な$_THEME_OPTIONS制御をリセットさせないよう修正する必要があるということになります。
親テーマ(子テーマ)のfunctions.phpに書くような、/lib/skin.phpで読み込まれるもの以外で$_THEME_OPTIONSを操作した場合なんですね。
とりあえず、以下のように修正してみることにしました。
https://github.com/yhira/cocoon/commit/628a5726807b18fe6269c280074c476d53230ddc
EME_OPTIONS
Topic starter
2019年8月19日 21:09
まず第一段階からということで、修正ありがとうございます。
早速確認しました。
https://github.com/yhira/cocoon/commit/628a5726807b18fe6269c280074c476d53230ddc
こちらの修正コードについてですが、私の#post-20816の回答にある
何らかの方法で設定変更前の$_THEME_OPTIONSから旧スキン制御部分のみリセットする必要がある
という説明内容が考慮されていないようです。
最初の修正で$_THEME_OPTIONSをリセットするコードを記述したのは、旧スキンにある$_THEME_OPTIONS制御を新スキンにある$_THEME_OPTIONS制御に書き換えることが目的だったと思います。
ただ、$_THEME_OPTIONS自体をリセットしてしまうとスキン以外の$_THEME_OPTIONS制御に影響が出る(例:#post-20824)ため、スキンにある$_THEME_OPTIONS制御のみを新しい設定でのスキンの制御に入れ替えるようコードを記述する必要があります。
わいひら reacted
2019年8月20日 22:08
ちょっと無理やりなんですか、以下のような形で旧スキン制御部分のみをリセット後、新スキン制御とマージして保存するようにしてみました。
https://github.com/yhira/cocoon/commit/564f5d2e27919fe047c50dfbf5fc95757cf2b7d4
https://github.com/yhira/cocoon/tree/skin_post_block
Topic starter
2019年8月21日 02:22
修正ありがとうございます。
再度修正後のものをインストールし、確認しているところです。
結果としては不具合が出ており、原因がまだ判明していない点・確認できていない点もあるのですが、ひとまず現状を報告させていただきます。
1. スキン内で関数を使用した場合の不具合
修正前(cocoon-master)では、_imports.phpまたはpage-settings/_top-page.phpで一度だけスキンのfunctions.phpを読み込む形でした。
スキンの変更が行われても、変更後のスキンfunctions.phpが読み込まれるだけなので、特に問題はありませんでした。
しかし、今回の修正によって少なくとも異なるスキン間で同じ関数名を使用し、異なる処理が書かれている場合に問題が出てきます。
(子テーマのfunctions.phpのような使い方も可能)
function_exists関数で関数の重複を防いでも、どの関数を実行用の関数として採用するかはスキン側のみでは制御するのは困難かと思います。
2. 保存処理後の$_THEME_OPTIONSがやはり一致しない場合が出てくる
こちらについてはまだどこが原因かは分かっていませんが、#post-20825にある例のような動作を除外した上(最初のCocoon設定ページ表示時の$_THEME_OPTIONSと保存処理を行うためのページ再読み込み時の$_THEME_OPTIONSが一致する形)で確認しても、保存処理後の$_THEME_OPTIONSが意図したものと異なる配列になってしまいます。
今のところ可能性として、
- 本来の$_THEME_OPTIONSがプログラムの実行に沿って適切な値のまま受け渡しできていない。
- $_THEME_OPTIONSが本来の値とは異なる段階でget_theme_option関数が実行されている。
などが考えられ、その辺を意識して原因を探っているところです。
修正コードも修正点も複雑化してきたので、再確認のために一旦本トピック立ち上げ時の【不具合】にある問題点まで戻って考えてみます。
各設定に対し、get_theme_option関数の実行タイミング以前に$_THEME_OPTIONSで値を指定すれば、指定した値が設定値となる仕組みでした。
(実際には、テーマ内の
https://github.com/yhira/cocoon/blob/master/lib/html-forms.php#L118
でも$_THEME_OPTIONSが用いられているが、判定のために取得しているだけなので無視できる。)
ただし、このまま素直に$_THEME_OPTIONSを制御すると一点だけ問題があり、$_THEME_OPTIONSがis_admin_php_page関数によってCocoon設定ページのみ干渉される点でした。
つまり、データとして保存される値の書き換えを無視すれば、$_THEME_OPTIONSは
function get_theme_option($option_name, $default = null){
//スキンにより固定値がある場合は採用する
$skin_option = get_skin_option($option_name);
if (!is_null($skin_option)) {
if ($skin_option == '0') {
$skin_option = 0;
}
return $skin_option;
}
return get_theme_mod($option_name, $default);
}
上記コードだけで成り立つシンプルな仕組みだということです。
そうすれば、$_THEME_OPTIONSの利用側はget_theme_option関数の実行タイミングを追うだけでよく、提供側は$_THEME_OPTIONSに干渉しないようget_theme_option関数で設定値を取得するだけでよいことになります。
残るは、データとして保存される値の書き換えを防ぐ修正になりますが、今の方向で修正していくと$_THEME_OPTIONSに干渉するポイントが増え、他のプログラムと合わさってより複雑化していくことになるかと思います。
それに伴い、$_THEME_OPTIONSの利用で不具合が生じたときに、不具合箇所の発見が難しくもなります。
どうしても修正負担が高いのと、新たにDBデータ取得用の関数でも書かない限り(もしくは引数で条件分岐とかしない限り)、get_theme_mod(OP_HEADER_LAYOUT_TYPE, 'center_logo')コードが2ヶ所重複してしまう
#post-20809
こちらでおっしゃっていることも理解できるのですが、はたして今行っている修正の方が上記よりも大きなメリットのある方法なのでしょうか?
Topic starter
2019年8月21日 17:33
私のカスタマイズ上での$_THEME_OPTIONSの挙動だけでなく、以下のような例でも確認してみました。
- 全ての設定値に対して、設定値と異なる変更値を用意し、_imports.php#L15の読み込み以前(例えばsetup_themeアクション)と以後(例えばafter_setup_themeアクション)それぞれのパターンで1か所の$_THEME_OPTIONSにまとめて記述(それぞれ$_THEME_OPTIONS_BEFORE、$_THEME_OPTIONS_AFTERとする)。
- valueの選択が3つ以上ある一部の設定値に対して、設定値とも1の変更値とも異なる変更値を用意し、スキンfunctions.phpの$_THEME_OPTIONSに記述($_THEME_OPTIONS_SKINとする)。
- 管理画面上での設定値の変更がない状態で、最初のCocoon設定ページ読み込み時の$_THEME_OPTIONS(…※1)と保存処理後の$_THEME_OPTIONS(…※2)を比較。
- _imports.php#L15を基準として、以前のパターンでは$_THEME_OPTIONS_BEFOREに$_THEME_OPTIONS_SKINを上書きしたもの、以後のパターンでは$_THEME_OPTIONS_SKINに$_THEME_OPTIONS_AFTERを上書きしたものが※1・※2両方の$_THEME_OPTIONSと一致するべきだが、どちらのパターンでも※2は一致しない。
- 保存処理後に新たにスキンの$_THEME_OPTIONSを読み込む点を考慮して、保存処理後のみ「$_THEME_OPTIONS_SKINに$_THEME_OPTIONS_AFTERを上書き」を逆($_THEME_OPTIONS_AFTERに$_THEME_OPTIONS_SKINを上書き)にした場合の$_THEME_OPTIONSと比較しても※2が一致しない。
以前のパターンで一致しない、5の場合でも一致しないとなるとやはり$_THEME_OPTIONSの受け渡し方に問題があると言えると思うので、
https://github.com/yhira/cocoon/blob/skin_post_block/lib/page-settings/_top-page.php#L18-L148
の一連、特に複数回リセットしている辺りを重点的に見直す必要がありそうです。
ひとつ前の返信にある【不具合】の問題点について補足を加えておきます。
$_THEME_OPTIONSの利用側はget_theme_option関数の実行タイミングを追うだけでよく、提供側は$_THEME_OPTIONSに干渉しないようget_theme_option関数で設定値を取得するだけでよいことになります。
Cocoonでは各設定値をもとにプログラムが動作するので、設定画面上で用意されている設定値($_THEME_OPTIONSがない状態)で動作に不具合がない限り、$_THEME_OPTIONSの利用で意図したように動作しない場合は問題点を「get_theme_option関数への$_THEME_OPTIONSの受け渡し」という1点に絞ることができます。
(get_theme_option関数実行時の$_THEME_OPTIONSに指定した変更値が入っているか?)
指定した変更値がある場合はCocoonのプログラムが求めていない値を変更値にしてしまっていることになり、指定した変更値でない場合はget_theme_option関数の実行タイミングと$_THEME_OPTIONSの記述タイミングを比較すればよいことになります。
修正点として私が本トピック立ち上げ時に希望したことは、
function get_theme_option($option_name, $default = null){
//スキンにより固定値がある場合は採用する
$skin_option = get_skin_option($option_name);
if (!is_null($skin_option)) {
if ($skin_option == '0') {
$skin_option = 0;
}
return $skin_option;
}
return get_theme_mod($option_name, $default);
}
のようにis_admin_php_page関数をget_theme_option関数から取り除き、$_THEME_OPTIONSがget_theme_option関数と1対1のシンプルなものになることでした。
グローバル変数の性質から考えても、不具合箇所を絞りやすい方が良いかと思います。
cocoon-masterでは今のところis_admin_php_page関数の部分1点を除けば、Cocoon側ではget_theme_option関数も適切に扱われているようで、$_THEME_OPTIONSの利用に際して問題は出てきていません。
今の修正内容よりはis_admin_php_page関数の方が不具合を把握しやすいので、修正が難しいようでしたら、一旦保留でも構いません。
2019年8月21日 20:36
今回の修正は、「コードの大幅改修」の手間を省くとともに、Cocoon設定に「現在のスキン制御値」が出るというのは魅力ではありました。
これでうまくいけば問題なかったのですが、グローバル変数もう面倒くささが際立つ結果になってしまいました。
ところで1つ疑問なんですが、今回の変更は、こちらの子テーマのfunctions.phpで$_THEME_OPTIONSを変更することにより起こる不具合でした。
私の環境だと$_THEME_OPTIONSは動的制御が多く複雑なので、分かりやすく静的制御で例を示します。
global $_THEME_OPTIONS;
$_THEME_OPTIONS['amp_enable'] = 0;例えば、上記を子テーマfunctions.phpにでも記述するとしましょう。
最初のCocoon設定ページ読み込み時には反映されるものの、
https://github.com/yhira/cocoon/blob/skin_post_block/lib/page-settings/_top-page.php#L112-L113
リンク先のコードによって$_THEME_OPTIONSそのものがリセットされるため、保存実行後の設定ページでは反映されない、ということです。
つまり、修正の第一段階としては、必要な$_THEME_OPTIONS制御をリセットさせないよう修正する必要があるということになります。
Cocoon自体元々は、「スキンのfunctions.php」と「スキンのoption.csv」と「スキンのoption.json」でスキン制御行うつもりではありました。
なので、以下の変更を行う前の状態に戻して
https://github.com/yhira/cocoon/commit/564f5d2e27919fe047c50dfbf5fc95757cf2b7d4
今後$_THEME_OPTIONSの変更は、上記3ファイルに絞る仕様にしたとしても、何かしら不具合はあったということでしょうか?
2019年8月21日 20:36
もちろん、最初のご提案でき解決するのは分かります。
ただ、コードの重複を避けつつ全てを書き換えるとなると、それなりの手間がかかるため、できるだけ負担の少ないやり方を探るというのは、プログラムを行う上で重要なのでご容赦ください。
もし当初の方法をやるとするならば、できるだけ負担の少ない書き換え方法を今度は探ります。
ただ、やはり数が多いため、すぐにはできないと思います。
なので、編集時間もあるのでしばらく保留状態にはなると思います。
2019年8月21日 20:38
Topic starter
2019年8月21日 22:14
ひとつひとつ回答していきます。
今回の変更は、こちらの子テーマのfunctions.phpで$_THEME_OPTIONSを変更することにより起こる不具合でした。
...
今後$_THEME_OPTIONSの変更は、上記3ファイルに絞る仕様にしたとしても、何かしら不具合はあったということでしょうか?
これは分かりやすい例として示しただけで、それをスキンの
- functions.php
- option.csv
- option.json
のみに制限しても問題が生じることに変わりはありません。
(または個々の設定値について検証した上で、「これには変更値を使用できない」「このパターンはダメ」などルールを設けていくしかない。)
#post-20896の2でも説明してあるように、$_THEME_OPTIONSの適切な受け渡しと正しい$_THEME_OPTIONSでget_theme_option関数が実行される必要があります。…※
$_THEME_OPTIONSの流れとget_theme_option関数の関係を把握してもらうためにも、一旦スキン以外(子テーマ)に$_THEME_OPTIONSを書いた例を示したつもりでした。
以下の変更を行う前の状態に戻して
https://github.com/yhira/cocoon/commit/564f5d2e27919fe047c50dfbf5fc95757cf2b7d4
こちらだと複数回lib/skin.phpを読み込んでいるので、#post-20896の1もやはり問題になってくるのではないでしょうか。
そして後回しになっていますが、#post-20825にある問題についても何も解決できていません。
個人的な意図した動作にならない問題だけならまだしも、こちらはデータベースに直接影響する点なので、そのままだとセキュリティリスクになるのではないでしょうか。
なので、$_THEME_OPTIONS自体にもセキュリティ対策を講じるか、データベースに保存されないようにするかが必要になると思います。
(そのために、修正の第一段階である※を解決しなければならない。)
私はそういった諸々の点も考慮した上で、修正が入るのであれば$_THEME_OPTIONSとデータベース周りのコードは切り離した方がいいと考え、多少修正箇所は増えてしまうもののシンプルで分かりやすい#post-20772のような1対1になる形での提案をしました。
はたして今行っている修正の方が上記よりも大きなメリットのある方法なのでしょうか?
このように申し上げたのは、「確かにわいひら様が修正するコード量、その負担を減らそうとしているのは分かるのですが、無理なコードを書いて問題点や不具合箇所を複雑化し、トレードオフとしてメンテナンス性を損ねていませんか?」という意味です。
不具合箇所をきちんと把握でき、今の修正方法が今後Cocoonを運用していくうえで扱いやすいのであれば異論はありません。
編集時間もあるのでしばらく保留状態にはなると思います。
最初に「できればテーマ側で解決してほしい」と申し上げた通り、テーマ側で解決していただけるのであれば、焦らなくてもいつでも大丈夫です。
以上の回答は飽くまでCocoon設定ページに限った内容であり、Cocoon設定ページ以外では$_THEME_OPTIONSを用いたカスタマイズはきちんと動作しています。
Topic starter
2019年8月21日 23:58
グローバル変数もう面倒くささが際立つ結果になってしまいました。
おそらく、問題全体をひとまとめにして考えているからではないでしょうか?
Cocoon自体元々は、「スキンのfunctions.php」と「スキンのoption.csv」と「スキンのoption.json」でスキン制御行うつもりではありました。
このような条件設定をしてしまうと、$_THEME_OPTIONS問題の本質を見失ってしまいそうな気がします。
(条件設定をするのであれば、その条件に則った仕様にするためのコードを書かないと問題点が曖昧になります。)
#post-20896や#post-20923の横ライン以降で色々まとめていますが、シンプルに考えさえすれば、
- $_THEME_OPTIONSの役割→Cocoon設定ページの設定値を変更するためのユーザー向けグローバル変数
- get_theme_option関数の役割→$_THEME_OPTIONSに変更値があれば変更値を、変更値がなければ設定値を返す、Cocoon側で$_THEME_OPTIONSの仕様を統一するための関数
- フォーム内フィールドの役割→データベースにある設定値を取得し、編集後のvalueをPOST、データベースに保存する処理の繰り返し
と処理ごとの役割に分担するだけで成り立ちます。
これらの役割を標準として考えると、
- get_theme_option関数にフィールドの設定値を取得する役割も持たせる
- $_THEME_OPTIONSに保存処理の実行を判定する役割も持たせる
みたいに異なる別の役割を複数持たせて複雑にした場合、新たな役割を制御するのと仕様を統一するためのプログラムも当然複雑になるので、必ずしも楽ができるわけではないということになります。
以上は、
Cocoon自体元々は、「スキンのfunctions.php」と「スキンのoption.csv」と「スキンのoption.json」でスキン制御行うつもりではありました。
の方向で色々と考えてみた結果の回答です。
2019年8月22日 22:42
このように申し上げたのは、「確かにわいひら様が修正するコード量、その負担を減らそうとしているのは分かるのですが、無理なコードを書いて問題点や不具合箇所を複雑化し、トレードオフとしてメンテナンス性を損ねていませんか?」という意味です。
それを試す意味での今回のブランチです(ダメな可能性が高かったので)。
試した上でダメなら、当然別の方法とります。
今回、get_theme_modで修正するとしたら、その修正箇所を数えてみたら、470以上になります(重複除外処理抜きで)。
さすがに、これだけの箇所を修正する必要が出てきたら、やはり多少おじけづいてしまいます。
そういう場合「少しでも楽な修正方法があるのではないか?」を考えるのは悪いことでは思います。ダメならやめればいいわけで。
ロコさんの修正案がダメと言っているわけでは決してなくて、出来る限り楽できる方法を探っていただけです(※今回の修正はダメそうでしたが)。
探った結果、難しいとなれば、それに固執するつもりもありません。
2019年8月22日 22:57
私の環境だと$_THEME_OPTIONSは動的制御が多く複雑なので、分かりやすく静的制御で例を示します。
修正の第二段階については主に動的制御が当てはまりますが、こちらも分かりやすく静的制御で説明を加えておきます。
こちらなどで書かれている、動的制御というのがどのような使い方をされているのか、いまいちピンときていないので、その後いくつか補足で質問してみた感じです。
どんな処理を行っているのか僕にしたらブラックボックスだったので。
これが想像できなかったので、説明とうまく結びつかなかったというか。
#post-20825についても、どんな動的処理を行ったら、そのような状態になるのかがわからず、とりあえず後回しにした次第です。
ただ、いずれにせよグローバル変数を制御するのは大変でありますし、不具合報告するのも難しいのは当然あると思うので、set_theme_mod保存時の処理はやらないと思います。
Topic starter
2019年8月23日 02:01
動的制御というのがどのような使い方をされているのか、いまいちピンときていない
グローバル変数そのままの扱い方ということです。
グローバル変数は1回のプログラムを通して、どのタイミングからでも、そして何度でも書き換えることができます。
しかし、動的な扱いをそのまま持ってくるとどのタイミングでどの値が入るのかを追っていく必要があり、問題の本質を見失う可能性があります。
そこで、動的な扱いを行った結果、最終的な$_THEME_OPTIONSを擬似的に静的変数として捉える形で例を示しました。
最終的な$_THEME_OPTIONS、要は変数の利用側が意図した値が入っている状態で、この状態をプログラム実行中保持される変数の起点と考えると静的な扱いと同等であるべきと言えます。
実際にはグローバル変数なので書き換えることが可能なのですが、もうこれ以上書き換えられないはずの値が入った変数を静的変数として見れば、書き換えることができてしまった場合におかしいことが分かります。
つまり、#post-20824や#post-20825のような例が正しく動作しない限りは不具合が残ったままだということです。
#post-20825の例にある値は、
https://github.com/yhira/cocoon/blob/skin_post_block_re/lib/page-settings/_top-page.php#L21
この保存処理の実行が開始される以前の最終的な値として捉えればよいので、保存処理の実行以前であればどのタイミングに書いてもらっても構いません。
条件分岐で書く、または
- Cocoon設定ページを開き、検証する設定箇所に値を入力、保存。
- 子テーマを開き、$_THEME_OPTIONSに設定箇所の異なる値を指定、ファイルの上書き保存。
- 再度Cocoon設定ページを開き、設定箇所が$_THEME_OPTIONSで指定した値であることを確認。
- 保存処理を行う前に違うタブで子テーマを開き、$_THEME_OPTIONSの記述を削除、ファイルの上書き保存。
- ファイルの上書きが完了したのを確認した後、設定の保存処理を開始。
- 保存処理が完了した後の設定項目の値を確認。
のような方法もあるかと思います。
これで処理前後のプログラム実行で$_THEME_OPTIONSが異なるパターンが再現できます。
$_THEME_OPTIONSの適切な受け渡しと正しい$_THEME_OPTIONSでget_theme_option関数が実行される必要があります。
#post-20951
重複しますが、上記の必要十分条件を満たすことが重要で、
https://github.com/yhira/cocoon/blob/skin_post_block_re/lib/_imports.php#L15
や
https://github.com/yhira/cocoon/blob/skin_post_block_re/lib/page-settings/_top-page.php#L14-L129
だけでなく、全体を通して色んなタイミングでget_theme_option関数が実行されていることも考慮しなくてはいけません。
仮に今の修正方向でいくとして色々検討はしましたが、ひとまず最初のCocoon設定ページ表示時の$_THEME_OPTIONSを、保存処理時の$_THEME_OPTIONSには持ち込まない形で書かないといけないとは思いました。
https://github.com/yhira/cocoon/blob/skin_post_block_re/lib/page-settings/_top-page.php#L116
リセット処理は行われないこと、lib/skin.phpの読み込みはプログラム全体を通して1回にすることが解決の道になるのかなと。
特にリセット処理が必要になるということは、その前後で完全に異なる$_THEME_OPTIONSになってしまっていると考えても相違ないと思います。
そうなると、最初の表示時の$_THEME_OPTIONSを次のプログラム実行へ引き継ぐ方法など、$_POSTに近い役割と言えるでしょうか、そういった機能を実装しないといけないのかもしれません。
あと、セキュリティ面からだと、$_THEME_OPTIONSはなるべく書き換えができないよう制限した方がよく、グローバル変数ではなく静的変数にして、lib/skin.php読み込みタイミングで指定された値に絞るようにすれば、入り口と出口の2箇所となり、対策はしやすくなります。
ただ、$_THEME_OPTIONSの自由度がほぼないに等しくなるので、直接関数を上書きするような書き方の方が便利なパターンが多くなり、実装する労力に見合うのかは疑問ですし、私自身このような実装を望んでいるわけではありません。
Topic starter
2019年8月23日 15:42
前回の返信に追記です。
とりあえずCocoon設定ページフォームの処理は置いておいて、「設定値を制御できる機能($_THEME_OPTIONS)」について再考するところから始めてはいかがでしょうか?
元々あった、$_THEME_OPTIONSに対する
function get_theme_option($option_name, $default = null){
//スキンにより固定値がある場合は採用する
$skin_option = get_skin_option($option_name);
if (!is_null($skin_option)) {
if ($skin_option == '0') {
$skin_option = 0;
}
return $skin_option;
}
return get_theme_mod($option_name, $default);
}
というシンプルな仕様でいくのかどうか。
具体的にわいひら様がどのような使われ方を求めているのかコードからでは理解できず、(is_admin_php_page関数による判定の問題はあったものの)私は元々あった上記の仕様に則ってコードを書いてきただけなので、その視点からしか修正方向を考えることができないのが現状です。
そして、この仕様に則って考えると、現在の修正は問題箇所をget_theme_option関数から保存処理周りに移し、より複雑化させて不具合パターンを増やしていっているだけなので、どのように不具合例を挙げればよいのかもよく分かっていません。
まずは、わいひら様が考えている仕様とコード上での仕様を一致させ、どこまでを仕様とし、どこからを不具合とするのか。
もとの仕様と、
Cocoon自体元々は、「スキンのfunctions.php」と「スキンのoption.csv」と「スキンのoption.json」でスキン制御行うつもりではありました。
で意図している仕様が異なるのであれば、飽くまで一例ですが、
$_THEME_OPTIONSはなるべく書き換えができないよう制限した方がよく、グローバル変数ではなく静的変数にして、lib/skin.php読み込みタイミングで指定された値に絞るようにすれば、入り口と出口の2箇所となり、対策はしやすくなります。
のように設定値の制御周りのコードを刷新し、仕様を再設定する必要があるかと思います。
2019年8月24日 20:13
スキンの仕様変更も含めて、ちょっと考えてみます。
Topic starter
2019年8月24日 21:58
ついでに動的な例も書いておくと、
Cocoonのstringを返すある設定'cocoon_setting_name'を動的に制御するとします。
そして、制御するプログラムファイル内にfunction1,function2があり、'cocoon_setting_name'に対してfunction1→function2→get_theme_optionの順で実行されるとします。
//default
global $_THEME_OPTIONS;
$_THEME_OPTIONS['cocoon_setting_name'] = 'Cocoon';
function function1() {
global $_THEME_OPTIONS;
$_THEME_OPTIONS['cocoon_setting_name'] = $_THEME_OPTIONS['cocoon_setting_name'].'の';
...
}
function function2() {
global $_THEME_OPTIONS;
if (is_category()) {
$_THEME_OPTIONS['cocoon_setting_name'] = $_THEME_OPTIONS['cocoon_setting_name'].'カテゴリーページ';
} elseif (is_single()) {
$_THEME_OPTIONS['cocoon_setting_name'] = $_THEME_OPTIONS['cocoon_setting_name'].'投稿ページ';
}
...
}
となっていれば、カテゴリーページで'cocoon_setting_name'がget_theme_option関数で返す値は'Cocoonのカテゴリーページ'となります。
(WordPressだとアクションフックを利用すれば、簡単に動作確認できます。)
全ての設定値に対し、上記のようにget_theme_option関数で返されるまでのどのタイミングで実行しても正しく変更値を返す仕様が
function get_theme_option($option_name, $default = null){
//スキンにより固定値がある場合は採用する
$skin_option = get_skin_option($option_name);
if (!is_null($skin_option)) {
if ($skin_option == '0') {
$skin_option = 0;
}
return $skin_option;
}
return get_theme_mod($option_name, $default);
}
というわけで、利用側はget_theme_option関数が実行されるまでに$_THEME_OPTIONSで最終的な変更値を決定すればよく、提供側は
- $_THEME_OPTIONSを利用側が指定できる配列とは異なる配列のときにget_theme_option関数を使用しない。
- プログラムで設定値を取得する際には必ずget_theme_option関数を使用し、$_THEME_OPTIONSに変更値がある場合は必ず変更値を返す。
の2点が守られてさえいれば成り立つと思います。
ただ、動的に制御する方法は無数にあるので、上記のような断片的な例だけではイメージしにくいです。
そこで、
//カテゴリーページ
$_THEME_OPTIONS = array('cocoon_setting_name' => 'Cocoonのカテゴリーページ');
//投稿ページ
$_THEME_OPTIONS = array('cocoon_setting_name' => 'Cocoonの投稿ページ');
//もし意図的に空にする処理があった場合
$_THEME_OPTIONS = array();
...
のように動的な制御を行った結果の最終的な$_THEME_OPTIONSだけを考え、get_theme_option関数が実行されるまでの全プログラム実行過程のどこで変更値が指定されても成り立てば、必然と動的な制御も成り立ちます。
Topic starter
2019年8月25日 18:18
Cocoon設定ページのみ読み込み、フィールドを生成する関数(generate_checkbox_tag、generate_radiobox_tag、generate_textbox_tagなど)が実行された場合のみ、その中で実行されるget_theme_option関数ではグローバル変数の参照を回避するプログラムをPHPの処理に組み込む形で実装したことで、一応今回の不具合を回避することに成功しました。
フィールド生成時かどうかの判定をget_theme_option関数にtrue/falseで受け渡す代わりに、裏でフィールド生成関数の実行を判定の材料にした形です。
ただ、PHPだけだと1対1にするにはやはり判定値を受け渡すとか、生のget_theme_mod関数で記述する必要がありそうなので、そのままの方法だとこれまでに示してきた案と変わりありません。
そこで、上記のプログラム実装時に思いついた別の案を示しておきます。
function get_theme_option($option_name, $default = null){
//スキンにより固定値がある場合は採用する
$skin_option = get_skin_option($option_name);
if (!is_null($skin_option)) {
if ($skin_option == '0') {
$skin_option = 0;
}
return $skin_option;
}
return get_theme_mod($option_name, $default);
}
まず、get_theme_option関数は$_THEME_OPTIONSに変更値がある場合はその変更値を返すようにします。
do_action('cocoon_settings_after_save');
https://github.com/yhira/cocoon/blob/master/lib/page-settings/_top-page.php#L117
設定の保存処理時にスキンの$_THEME_OPTIONSをマージする場合、マージ後に$_THEME_OPTIONSを扱える箇所を念のため用意しておきます。
($_THEME_OPTIONS利用側がフォーム読み込み時の書き換えを避けるための書き換え箇所)
https://github.com/yhira/cocoon/blob/master/lib/page-settings/_top-page.php#L204-L393
その上で、$_THEME_OPTIONSの影響を除外する対象をフォーム内の各フィールド読み込みのみに絞ります。
フォーム内の各フィールド読み込み時にCocoon側で使用するためのグローバル変数をもうひとつ($_FORM_OPTIONSとする)用意します。
以下、グローバル変数の宣言を省略します。
$_FORM_OPTIONS = $_THEME_OPTIONS;
$_THEME_OPTIONS = array();
https://github.com/yhira/cocoon/blob/master/lib/page-settings/_top-page.php#L203
フィールドの読み込み直前に、$_THEME_OPTIONSをCocoon側で使用する$_FORM_OPTIONSに移し、$_THEME_OPTIONSを空にします。
$_THEME_OPTIONS = $_FORM_OPTIONS;
https://github.com/yhira/cocoon/blob/master/lib/page-settings/_top-page.php#L394
読み込み終了時にもとに戻すことで、フィールド読み込み前後では$_THEME_OPTIONSは変わらず、フィールド読み込み時のみget_theme_option関数ではデータベースから直接取得するようになります。
これだとプレビュー画面にまで影響を与えてしまうので、プレビュー画面の読み込み時だけ$_THEME_OPTIONSをもとに戻すようにします。
$_THEME_OPTIONS = $_FORM_OPTIONS;
//プレビュー画面読み込み...
$_THEME_OPTIONS = array();
この処理を関数にしておけば、各プレビュー画面読み込み時の処理をまとめられると思います。
影響する点がもう1点あり、
function wrap_skin_control_tag($tag, $name){
if (get_skin_option($name) !== null ) {
$tag = get_skin_control_tag($tag);
}
return $tag;
}
フィールドに$_THEME_OPTIONS制御用タグをラップする処理です。
こちらはグローバル変数の参照元を$_THEME_OPTIONSから$_FORM_OPTIONSに変更するだけで対応できます。
wrap_skin_control_tag関数内のget_skin_option関数をget_form_option関数と変更し、
function get_form_option($name){
global $_FORM_OPTIONS;
if (isset($_FORM_OPTIONS[$name])) {
return $_FORM_OPTIONS[$name];
}
}
みたいな形にすることです。
実際に試してはいないのですが、おそらく以上でひとまずは対応できるような気がします。
修正点は数点+各プレビュー画面読み込み箇所で済みますし、個々のプレビュー画面読み込み時は毎回フォーム生成直前までの$_THEME_OPTIONSが参照されるようになります。
そして、$_THEME_OPTIONSの利用側は
https://github.com/yhira/cocoon/blob/master/lib/page-settings/_top-page.php#L204-L393
の読み込み時の書き換えを避けるようにすればよいのですが、この読み込み時に書き換えを当てはめられる箇所は今のところ「各フィールドタグ生成関数のフック」と「各プレビュー画面読み込み判定のフック」くらいしか思い当たらないので、基本的には影響なさそうです。
注意点があるとすれば、完全な1対1となる対応ではないため、Cocoon側でフィールド読み込み周りに何らかの仕様変更・追加が加えられた場合に成り立たなくなる可能性はあります。
Topic starter
2019年8月26日 14:24
上記の修正方法について補足しておくと、$_FORM_OPTIONSが$_THEME_OPTIONSのバックアップとして用いられることになります。
$_THEME_OPTIONS = $_FORM_OPTIONS;
...
$_THEME_OPTIONS = array();
をセットで考えるとリセットしているわけではなく、リストアを繰り返しているだけということです。
各フィールドでget_theme_option関数が実行される際は、
$_THEME_OPTIONS = array();
$_THEME_OPTIONSを空にしておくことで、get_theme_option関数をget_theme_mod関数と同等の処理で扱うことができます。
その他のget_theme_option関数が実行される箇所では、
$_FORM_OPTIONS = $_THEME_OPTIONS;
としておけば、本来の$_THEME_OPTIONSを考慮したget_theme_option関数が実行されます。
本来のget_theme_option関数が実行される数だけ
$_THEME_OPTIONS = $_FORM_OPTIONS;
...
$_THEME_OPTIONS = array();
の処理を行う必要が出てきますが、この処理を関数にしておけば、その関数を通す1行を加えるだけになります。
「各フィールドタグ生成関数のフック」と「各プレビュー画面読み込み判定のフック」くらいしか思い当たらないので、基本的には影響なさそうです。
「各プレビュー画面読み込み判定のフック」で$_THEME_OPTIONSを書き換えるくらいなら、プレビュー先のプログラム内で書き換えるべきであること、「各フィールドタグ生成関数のフック」で書き換えるとそもそも$_THEME_OPTIONS制御用ラップタグに影響してしまうカスタマイズ設計に問題があることから、上記のように"基本的には影響ない"と判断しました。
私が実際に行った修正方法とは異なるのですが、考え方としては近い形で、今のところ問題なく動作しており、利便性を保ったままシンプルなコードで解決できたので、とりあえずは今の形で運用していこうと思います。
2019年8月26日 20:23
このところ、グローバル変数を使用しない仕様に変更してみようかと、コードを書いてみてはいたのですが、それはそれで難しい部分がありました。
どうしてもPHPのみに絞ってスキン制御関数で変更する仕様に変更したとしても、「スキン→スキン」変更時にスキンファイル上で関数の二重読み込みになってしまう問題が解決できませんでした。
コードありがとうございます!
ちょっとこれから、ロコさんが書かれた方法を試してみたいと思います。
2019年8月26日 21:57
修正案を実装してみました。
https://github.com/yhira/cocoon/commit/878ef28462510f1954107761c54c1c21c450d551
問題なく動作しているように思います。
とりあえず現在、Cocoon設定の仕様変更の予定はないので、こちらでいかせていただこうかと思います。
Topic starter
2019年8月29日 15:55
修正ありがとうございます。
頭の中でコードを書いただけだったので何か不具合が出る可能性はありましたが、ざっと確認した限りでは問題は見当たらないようです。
どうしてもPHPのみに絞ってスキン制御関数で変更する仕様に変更したとしても、「スキン→スキン」変更時にスキンファイル上で関数の二重読み込みになってしまう問題が解決できませんでした。
おそらく、#post-20896の1と同様の問題かと思うのですが、設定値制御機能の仕様を制限してコード化しようとすると、最終的にスキンではPHPを使用できない設計になったのではないでしょうか?
わいひら reacted
2019年8月29日 21:01
ご確認ありがとうございます。
おそらく、#post-20896の1と同様の問題かと思うのですが、
そうです。
設定値制御機能の仕様を制限してコード化しようとすると、最終的にスキンではPHPを使用できない設計になったのではないでしょうか?
そうなりました。
スキンのfunctions.phpで関数名を書いて関数を定義できなくなりました。
設定保存時と、Cocoon設定に新しいスキン制御を反映させるため、
必ず新旧スキンのfunctions.phpを2回読み込まなくてはならず、関数名の重複で、結局使用できない設計になりました。
Topic starter
2019年8月30日 15:27
やはりそうだったのですね。
#post-20943でグローバル変数をなくすだけでなく、スキンでのPHPを使用不可にする仕様変更が見え、Cocoonの仕様が今の仕様とどちらに向かうかによって話が全然変わってくると思ったので、#post-21031で仕様の再考をお願いした次第です。
それと並行して、$_THEME_OPTIONS機能を独立させ、カスタマイズ側のみで実装する形を探った結果から#post-21135の修正案が出ました。
少し遠回りになってしまいましたが、対応ありがとうございました。
2019年8月30日 17:28
#post-20943でグローバル変数をなくすだけでなく、スキンでのPHPを使用不可にする仕様変更が見え
いえいえ、この時点では、仕様変更するつもりは全然なかったです。
#post-21031で仕様の再考をお願いした次第です。
こちらで、仕様の再考提案があってから、「グローバル変数を使用しないとしたらどうなるか」とブランチを切って検討しただけです。
#post-21101を書いたあと、ブランチを切ってPHPのみでスキン制御を行う仕様変更を試していて、ダメだったので、#post-21135を試した感じです。
一応その時試していたボツコードがこちら。
https://github.com/yhira/cocoon/commits/get_skin_theme_options
固定ページ 1 / 2
次へ
問題の解決に至った場合には、トピック冒頭の「解決済み」をクリックしていただけますと幸いです。
また、有用な回答があった場合は返信右下にある「いいね!」もご活用ください。回答者の励みになります。
(CC BY-ND 2.1)準じていれば(リンクを貼っていただければ)転載も自由です。カスタマイズ記事を書く際にコード等をコピペ利用していただいて構いません。
フォーラムの使い方がよくわからない場合は、テストトピックで自由にテストしていただいて構いません。
最近の書き込みはこちら。
詳細なカスタマイズ依頼をするならこちら。