Opened 22 months ago

Closed 20 months ago

Last modified 20 months ago

#6292 closed change (fixed)

Make issue reporter compatible with test server

Reported by: greiner Assignee: saroyanm
Priority: P2 Milestone:
Module: User-Interface Keywords:
Cc: agiammarchi, saroyanm Blocked By:
Blocking: Platform: Unknown / Cross platform
Ready: yes Confidential: no
Tester: Unknown Verified working: no
Review URL(s):

https://codereview.adblockplus.org/29716600/

Description

Background

We've been using a basic test server to be able to work and test UI pages without having to worry about compatiblity with the extension. For that we use a mock implementation of extension. The newly introduced issue reporter, however, isn't yet compatible with that mock which means that we cannot work with it on the test server.

What to change

  • Fix errors in JavaScript console of issue-reporter.html by adding missing scripts which are used to establish the background page as a subframe (see other UI pages for reference)
  • Verify that the introduction of new scripts doesn't cause issues when the UI is bundled with the extension since the equivalent scripts in adblockpluschrome may behave slightly differently
  • Ensure that browser.* calls in issue-reporter.js don't fail and otherwise add the missing functionality to background.js

Change History (9)

comment:1 Changed 21 months ago by saroyanm

  • Owner set to saroyanm

comment:2 Changed 21 months ago by saroyanm

  • Priority changed from P3 to P2

Increased the prio:
In order to avoid issues during the issue reporter filter hits data implementation, I think would be great to fix this first, to make the actual UI of the implementation testable in the ABPUI.

comment:3 Changed 21 months ago by saroyanm

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

comment:4 Changed 20 months ago by abpbot

A commit referencing this issue has landed:
Issue 6292 - Make issue reporter compatible with test server

comment:5 Changed 20 months ago by saroyanm

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

comment:6 follow-up: Changed 20 months ago by Ross

Done. There are a few issues:

  • How the logo/page title are displayed at the top just looks... strange.
  • The Continue/Cancel buttons have no text/labels. Guessing this is because loading the translation json 404's.

Sending a report fails, but I'm guessing it's supposed to?

comment:7 in reply to: ↑ 6 ; follow-ups: Changed 20 months ago by saroyanm

Replying to Ross:
Thanks for the notes Ross.

Done. There are a few issues:

  • How the logo/page title are displayed at the top just looks... strange.

Until it's not more strange than it's defined, it should be fine.

  • The Continue/Cancel buttons have no text/labels. Guessing this is because loading the translation json 404's.

It's because current Issue reporter page is using strings from the adblockpluschrome repository, this strings needs to be migrated first, or we might translate them, so it's expected that the strings are not working well.

Sending a report fails, but I'm guessing it's supposed to?

Yes, that would be expected.

Anyway this shouldn't be blocker for the release until actual issue reporter works as it used to work.

comment:8 in reply to: ↑ 7 Changed 20 months ago by saroyanm

Replying to saroyanm:

Replying to Ross:

  • The Continue/Cancel buttons have no text/labels. Guessing this is because loading the translation json 404's.

It's because current Issue reporter page is using strings from the adblockpluschrome repository, this strings needs to be migrated first, or we might translate them, so it's expected that the strings are not working well.

Just created an issue for addressing this #6554

comment:9 in reply to: ↑ 7 Changed 20 months ago by greiner

Replying to saroyanm:

Replying to Ross:

Sending a report fails, but I'm guessing it's supposed to?

Yes, that would be expected.

FYI: I've created gitlab#55 to fix that.

Note: See TracTickets for help on using tickets.