Opened 9 months ago

Closed 9 months ago

Last modified 9 months ago

#7222 closed defect (fixed)

Copyright notice missing from first run page on Edge

Reported by: kzar Assignee: kzar
Priority: P1 Milestone:
Module: User-Interface Keywords:
Cc: sebastian, geo, greiner, Ross, oleksandr, agiammarchi Blocked By:
Blocking: #7233 Platform: Edge
Ready: yes Confidential: no
Tester: Ross Verified working: yes
Review URL(s):

https://gitlab.com/eyeo/adblockplus/abpui/adblockplusui/merge_requests/198

Description (last modified by kzar)

Environment

Microsoft Edge 42.17134.1.0, Adblock Plus 0.9.12

How to reproduce

  1. Install Adblock Plus
  2. Scroll to bottom of the first run page which opens.

Observed behaviour

The copyright notice text is missing from the grey footer.

Expected behaviour

That text should be populated.

Notes

  • This does not seem to be a recent regression, but without knowing the revision 0.9.11 was based upon I can't check if this is a regression since the previous release.
  • It seems that the text is not being populated for some reason, despite it working on Chrome and despite the other text on the page being populated OK.
  • See this screenshot.

There are two issues, both in js/first-run.js:

  1. The callback for api.doclinks.get("eyeo") never happens.
  2. browser.i18n.getMessage expects the arguments to be strings, but we were passing the year as a number.

This bodge of a patch got the copyright notice to display for me (Edge 42), but obviously now we need to figure out how to get api.doclinks.get("eyeo") to work properly

  • js/first-run.js

    diff --git a/js/first-run.js b/js/first-run.js
    index 513a63f..7ca9ce4 100644
    a b function initLinks() 
    5454  { 
    5555    ext.i18n.setElementLinks("fair-description", url); 
    5656  }); 
    57   api.doclinks.get("eyeo").then((url) => 
     57  Promise.resolve("https://adblockplus.org/redirect?link=eyeo&lang=en-US").then((url) => 
    5858  { 
    59     const year = new Date().getFullYear(); 
     59    const year = new Date().getFullYear().toString(); 
    6060    const notice = document.getElementById("copyright-notice"); 
    6161    ext.i18n.setElementText(notice, "firstRun_footer_copyright", year); 
    6262    ext.i18n.setElementLinks("copyright-notice", url); 

For Geo (Edge 44), the callback is already happening, so only the .toString() part of the above patch is required.

See also ui#299.

Change History (13)

comment:1 Changed 9 months ago by kzar

  • Description modified (diff)

comment:2 Changed 9 months ago by kzar

  • Component changed from Platform to User-Interface
  • Priority changed from P2 to Unknown
  • Ready unset

Anyway, it seems like this is a UI issue so I've un-triaged it. Could you triage please Thomas?

comment:3 Changed 9 months ago by greiner

  • Description modified (diff)
  • Priority changed from Unknown to P1
  • Ready set

comment:4 Changed 9 months ago by kzar

  • Cc agiammarchi added

comment:5 Changed 9 months ago by kzar

  • Description modified (diff)

comment:6 Changed 9 months ago by kzar

  • Owner set to kzar

Investigating how to get the api.doclinks.get("eyeo") part to work on Edge 42 now.

comment:7 Changed 9 months ago by kzar

  • Review URL(s) modified (diff)
  • Status changed from new to reviewing

comment:8 Changed 9 months ago by greiner

  • Resolution set to fixed
  • Status changed from reviewing to closed

comment:9 Changed 9 months ago by greiner

  • Blocking 7233 added

comment:11 Changed 9 months ago by abpbot

A commit referencing this issue has landed:
Issue 7222 - Fix the first run page copyright notice on Edge

comment:12 Changed 9 months ago by abpbot

comment:13 Changed 9 months ago by Ross

  • Tester changed from Unknown to Ross
  • Verified working set

Fixed. Copyright notice is back.

0.9.12.2237
Edge 44.17763.1.0 / EdgeHTML 18.17763 / Windows 10

Note: See TracTickets for help on using tickets.