Opened on 01/19/2018 at 05:38:48 PM
Closed on 03/13/2018 at 11:20:35 AM
Last modified on 04/05/2018 at 02:30:16 PM
#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): | |||
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
Attachments (0)
Change History (9)
comment:1 Changed on 03/07/2018 at 10:52:22 AM by saroyanm
- Owner set to saroyanm
comment:2 Changed on 03/07/2018 at 11:09:55 AM by saroyanm
- Priority changed from P3 to P2
comment:3 Changed on 03/07/2018 at 04:17:35 PM by saroyanm
- Review URL(s) modified (diff)
- Status changed from new to reviewing
comment:4 Changed on 03/13/2018 at 11:19:05 AM by abpbot
A commit referencing this issue has landed:
Issue 6292 - Make issue reporter compatible with test server
comment:5 Changed on 03/13/2018 at 11:20:35 AM by saroyanm
- Resolution set to fixed
- Status changed from reviewing to closed
comment:6 follow-up: ↓ 7 Changed on 04/05/2018 at 10:44:34 AM 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: ↓ 8 ↓ 9 Changed on 04/05/2018 at 10:58:14 AM 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 on 04/05/2018 at 11:07:23 AM 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 on 04/05/2018 at 02:30:16 PM 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.
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.