Skip to content
This repository has been archived by the owner on Feb 20, 2023. It is now read-only.

Properly quote descriptions in ad metric definitions #19243

Merged
merged 1 commit into from
Apr 26, 2021

Conversation

wlach
Copy link
Contributor

@wlach wlach commented Apr 26, 2021

This is pedantic, but strictly something called <provider-name> is considered an HTML tag
unless it's in a code block (backticks). This can cause problems with downstream consumers like the glean dictionary unless we work around it there:

image

See mozilla/glean-dictionary#549 and mozilla/glean-dictionary#497. I'm working around this downstream in the glean dictionary now (I suspect this is going to crop up again and don't want to go around fixing this everywhere) but figured I might as well file a PR here while I was at it.

FWIW you can view this metric here: https://dictionary.protosaur.dev/apps/fenix/metrics/browser_search_ad_clicks

Pull Request checklist

  • Tests: This PR includes thorough tests or an explanation of why it does not
  • Screenshots: This PR includes screenshots or GIFs of the changes made or an explanation of why it does not
  • Accessibility: The code in this PR follows accessibility best practices or does not include any user facing features. In addition, it includes a screenshot of a successful accessibility scan to ensure no new defects are added to the product.

To download an APK when reviewing a PR:

  1. click on Show All Checks,
  2. click Details next to "Taskcluster (pull_request)" after it appears and then finishes with a green checkmark,
  3. click on the "Fenix - assemble" task, then click "Run Artifacts".
  4. the APK links should be on the left side of the screen, named for each CPU architecture

This is pedantic, but strictly something called <provider-name> is considered an HTML tag
unless it's in a code block (backticks). 

See mozilla/glean-dictionary#549 and mozilla/glean-dictionary#497. I'm going to fix this upstream
but figured I might as well file a PR here to fix the underlying issue.
@wlach wlach requested review from a team as code owners April 26, 2021 15:19
@firefoxci-taskcluster
Copy link

No Taskcluster jobs started for this pull request
The `allowPullRequests` configuration for this repository (in `.taskcluster.yml` on the
default branch) does not allow starting tasks for this pull request.

@gabrielluong gabrielluong self-assigned this Apr 26, 2021
@gabrielluong gabrielluong added pr:approved PR that has been approved pr:needs-landing PRs that are ready to land [Will be merged by Mergify] labels Apr 26, 2021
@codecov-commenter
Copy link

Codecov Report

Merging #19243 (68472a7) into master (eec6411) will decrease coverage by 0.00%.
The diff coverage is 41.30%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master   #19243      +/-   ##
============================================
- Coverage     34.84%   34.84%   -0.01%     
+ Complexity     1653     1652       -1     
============================================
  Files           543      543              
  Lines         22006    22031      +25     
  Branches       3295     3301       +6     
============================================
+ Hits           7669     7677       +8     
- Misses        13430    13445      +15     
- Partials        907      909       +2     
Impacted Files Coverage Δ Complexity Δ
...org/mozilla/fenix/components/BackgroundServices.kt 20.83% <ø> (ø) 0.00 <0.00> (ø)
.../mozilla/fenix/search/awesomebar/AwesomeBarView.kt 0.00% <0.00%> (ø) 0.00 <0.00> (ø)
...x/settings/creditcards/CreditCardEditorFragment.kt 0.00% <0.00%> (ø) 0.00 <0.00> (ø)
...editcards/controller/CreditCardEditorController.kt 33.33% <ø> (ø) 0.00 <0.00> (ø)
...editcards/interactor/CreditCardEditorInteractor.kt 100.00% <ø> (ø) 0.00 <0.00> (ø)
.../settings/creditcards/view/CreditCardEditorView.kt 95.91% <88.23%> (-4.09%) 8.00 <0.00> (ø)
...src/main/java/org/mozilla/fenix/components/Core.kt 21.08% <100.00%> (+0.47%) 3.00 <0.00> (-1.00) ⬆️
...enix/settings/creditcards/CreditCardEditorState.kt 100.00% <100.00%> (ø) 7.00 <0.00> (ø)
...tings/creditcards/view/CreditCardItemViewHolder.kt 100.00% <100.00%> (ø) 3.00 <1.00> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 883cb7c...68472a7. Read the comment docs.

@gabrielluong gabrielluong merged commit 8070a32 into mozilla-mobile:master Apr 26, 2021
mcarare added a commit to mcarare/fenix that referenced this pull request Apr 27, 2021
pkirakosyan pushed a commit to gexsi/user-agent-android that referenced this pull request Aug 10, 2021
…9243)

This is pedantic, but strictly something called <provider-name> is considered an HTML tag
unless it's in a code block (backticks). 

See mozilla/glean-dictionary#549 and mozilla/glean-dictionary#497. I'm going to fix this upstream
but figured I might as well file a PR here to fix the underlying issue.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
pr:approved PR that has been approved pr:needs-landing PRs that are ready to land [Will be merged by Mergify]
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants