Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

GMOSSP Bid Adapte:Add user module, meta_url. #5

Closed
wants to merge 2 commits into from
Closed

GMOSSP Bid Adapte:Add user module, meta_url. #5

wants to merge 2 commits into from

Conversation

matsumoto-kouichi
Copy link
Owner

@matsumoto-kouichi matsumoto-kouichi commented Jan 26, 2022 •
edited
Loading

テスト準備

  • gulp serveしてprebid.jsを生成し、http://localhost:9999/build/dev/prebid.js でアクセスが確認できたら、
    prebid.jsをテストURLで確認できるよう下記のようにコピーする。
    scp ~/Prebid-fork.js/build/dev/prebid.js st01:/var/www/matsumoto/gmossp-ad/public/adtest/hb/prebid-matsumoto.js

  • テストページのquery.phpを編集して、下記のタグでHBを呼び出します
    https://gmomobile.backlog.jp/view/GMOAM-2975

    pixelEnabled: true,
    をタグに追加しないと、既存のGmosspAdapter.jsのgetUserSyncsでは、UserSyncされなそう

テスト

https://dev.sp.gmossp-sp.jp/hb/prebid/query.ad?tid=0409169b-f779-412e-8aa5-a543273e1e01&bid=2c0e6b65427667&ver=6.10.0-pre&sid=83494&im_uid=i.50x7JqK9SseigEndGYGuKg&url=https%3A%2F%2Fdev.sp.gmossp-sp.jp%2Fadtest%2Fhb%2Fquery.php%3Fdev%3Dmatsumoto%26space_id%3D83494&meta_url=https%3A%2F%2Ftrilltrill.jp%2Farticles%2F1935100&cur=JPY&dnt=0&

関連タスク

https://gmomobile.backlog.jp/view/GMOAM-1812

Type of change

  • Bugfix
  • Feature
  • New bidder adapter
  • Code style update (formatting, local variables)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • CI related changes
  • Does this change affect user-facing APIs or examples documented on http://prebid.org?
  • Other

Description of change

Supports imuid module , sharedId module , identityLink module.
PR on the docs repo: https://github.com/prebid/prebid.github.io/pull/
Supports canonicalUrl, og:url.

  • test parameters for validating bids
    {
    bidder: 'gmossp',
    params: {
    sid: '61590'
    }
    }
  • contact email of the adapter's maintainer
    [email protected]
  • official adapter submission

For any changes that affect user-facing APIs or example code documented on http://prebid.org, please provide:

Other information

@KazuakiM KazuakiM force-pushed the add_user_module_meta_url_safeframe branch from 131f44b to 2029f98 Compare January 27, 2022 05:15
KazuakiM
KazuakiM requested changes Jan 27, 2022
View reviewed changes
modules/gmosspBidAdapter.js Outdated
@@ -167,4 +190,9 @@ function getReferrer() {
}
}

function isSafeFrameWindow() {
const ws = getWindowSelf();
return !!(ws.$sf && ws.$sf.ext);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ws.$sf.ext のチェックは不要だと思います。
ws.$sf があるか、ないかだけで大丈夫かと思います。

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

修正しました。

modules/gmosspBidAdapter.js Outdated
@@ -141,9 +145,28 @@ function getCurrencyType() {
}

function getUrlInfo(refererInfo) {
let canonicalLink = null;
let url = getUrl(refererInfo);
let canonicalLinkContainer = window.top.document.querySelector("link[rel='canonical']");// html element containing the canonical link
Copy link
Collaborator

@KazuakiM KazuakiM Jan 27, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

後ろのコメントは不要だと思います。
window.top は悩ましいです。 og:url の方も含めて悩ましいです。
try - catch で window.top 非対応のときもフォローしたほうが良いように思います。

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

そうですね。定義してあるケースは、window.topしかないと思うので、相当悩みました。

window.topサポートしてないブラウザはなさそうですけど、エラーになるケースってあるんですかね・・
全て、try catchを指定するのも微妙だな〜と思いますが、他で入っているので仕方なく入れますか。

Copy link
Collaborator

@KazuakiM KazuakiM Jan 28, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

iframeにくくわれている際にエラーとなると思います。
将来的にはSafeFrame、AMPも対応するかもです。
AMPは廃れるので放置で良いと思いますが、SafeFrameですね、問題は。
一応このBidAdapterのポリシーとしては「SafeFrameかどうかの判定処理を入れている」つまり、「SafeFrame配信できる」ようなBidAdapterというコードにしておきたいです。

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SafeFrameでリファラがドメインだけになっているなどありますが、
SafeFrameという情報がわかる事で新たなアクションを起こせたらと考えています。

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

こちら、修正しました。

KazuakiM
KazuakiM requested changes Feb 1, 2022
View reviewed changes
modules/gmosspBidAdapter.js Outdated
return {
url: getUrl(refererInfo),
url: url,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

修正前のママで良さそうです。

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

元に戻しました。

KazuakiM
KazuakiM reviewed Feb 1, 2022
View reviewed changes
Copy link
Collaborator

@KazuakiM KazuakiM left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

メモ：

npx gulp build --modules=gmosspBidAdapter,imuIdSystem,sharedIdSystem,identityLinkIdSystem
cp $HOME/src/github.com/matsumoto-kouichi/Prebid.js/build/dist/prebid.js $HOME/src/github.com/gmo-am/gmossp-ad/public/adtest/hb/prebid-mabuchi.js

KazuakiM
KazuakiM reviewed Feb 1, 2022
View reviewed changes
Copy link
Collaborator

@KazuakiM KazuakiM left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

下記を設定して画面描画した所、リクエストパラメータが期待したものではありませんでした。ご確認をお願い致します。

<link rel="canonical" href="https://trilltrill.jp/articles/1935100" />

KazuakiM
KazuakiM reviewed Feb 1, 2022
View reviewed changes
Copy link
Collaborator

@KazuakiM KazuakiM left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ss

@KazuakiM KazuakiM force-pushed the add_user_module_meta_url_safeframe branch from 001df78 to 67b07a0 Compare February 21, 2022 05:51
@KazuakiM KazuakiM changed the title Add user module meta url safeframe GMOSSP Bid Adapte:Add user module, meta_url. Feb 21, 2022
KazuakiM
KazuakiM approved these changes Feb 22, 2022
View reviewed changes
Copy link
Collaborator

@KazuakiM KazuakiM left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

matsumoto-kouichi pushed a commit that referenced this pull request Mar 4, 2022
@patmmccann
Merge pull request #5 from prebid/master
7f186df 
rebase
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@KazuakiM KazuakiM KazuakiM approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@matsumoto-kouichi @KazuakiM