Opened 9 months ago

Closed 4 weeks ago

#6936 closed change (fixed)

Update adblockplusui dependency to 085509a03c94 (release-2018-5)

Reported by: greiner Assignee: kzar
Priority: P2 Milestone: Adblock-Plus-for-Chrome-Opera-Firefox-next
Module: Platform Keywords:
Cc: wspee, sebastian, rscott, Ross, philll, ukacar Blocked By: #5960, #6323, #6324, #6328, #6397, #6641, #6765, #6875, #6880, #6951, #6952, #6968, #6973, #6984, #6995, #7005, #7022, #7023, #7066, #7091, #7109, #7110, #7118, #7134, #7139, #7140, #7144, #7148, #7175, #7229, #7231, #7234, #7239, #7247, #7251, #7264, #7288, #7292, #7300, #7304, #7306, #7327, #7345, #7357, #7439, #7472
Blocking: Platform: Unknown / Cross platform
Ready: yes Confidential: no
Tester: Unknown Verified working: no
Review URL(s):

https://gitlab.com/eyeo/adblockplus/adblockpluschrome/merge_requests/71
https://gitlab.com/eyeo/adblockplus/adblockpluschrome/merge_requests/72
https://gitlab.com/eyeo/adblockplus/adblockpluschrome/merge_requests/74

Description (last modified by kzar)

Background

This imports the following changes:

Ticket Status Resolution Summary Component Owner
#5960 closed fixed Fix incorrect source strings in "mobile options" project User-Interface greiner
#6323 closed fixed Move composer.html related files to adblockplusui User-Interface greiner
#6324 closed fixed Move messageResponder.js to lib directory User-Interface greiner
#6328 closed fixed Move all icons into skin/icons directory in adblockplusui User-Interface greiner
#6397 closed fixed Implement language dropdown User-Interface agiammarchi
#6641 closed fixed Copyright year in about dialog is out of date User-Interface greiner
#6765 closed rejected No COPYING file in the root directory Platform
#6875 closed fixed Implement IOFilterList with infinite scroll User-Interface agiammarchi
#6880 closed fixed Make DevTools panel translatable User-Interface greiner
#6951 closed fixed Nightly builds don't include bundles User-Interface wspee
#6952 closed fixed Build Nightlies to feature branches User-Interface wspee
#6968 closed fixed Remove unused translatable strings from popup.json User-Interface greiner
#6973 closed fixed Open rewritten URLs in resources DevTools panel User-Interface greiner
#6984 closed fixed Remove duplicate gear icons from adblockplusui User-Interface greiner
#6995 closed fixed Add chrome builds to ABP UI Nightlies User-Interface wspee
#7005 closed fixed ABP UI Nightlies are missing peer dependencies User-Interface wspee
#7022 closed fixed Automatically use latest matching branch of adblockpluschrome for nightlies User-Interface wspee
#7023 closed fixed Add warning to issue reporter for screen readers User-Interface greiner
#7066 closed fixed Homepage meta data is ignored from some subscriptions User-Interface greiner
#7091 closed fixed Create font generation tool User-Interface saroyanm
#7109 closed fixed Add default characters to the font generations process User-Interface saroyanm
#7110 closed fixed Only show bubble UI notification for pages that match URL filters User-Interface greiner
#7118 closed fixed Remove duplicated trash icon User-Interface agiammarchi
#7134 closed fixed Firefox `event.layerX` coordinates off by `element.offsetLeft` User-Interface agiammarchi
#7139 closed fixed Chrome nightlies unuseable due to non numerical version number User-Interface wspee
#7140 closed fixed CI Pipeline succeeds despite linting errors User-Interface agiammarchi
#7144 closed fixed Fix issue reporter mock User-Interface greiner
#7148 closed fixed Change extension description used in manifest User-Interface wspee
#7175 closed fixed Issue reporter is broken on Edge User-Interface agiammarchi
#7229 closed fixed Whitelisted websites list not alphabetically sorted User-Interface greiner
#7231 closed fixed Cumulative fixes for io-filter-search User-Interface agiammarchi
#7234 closed fixed Cumulative fixes for io-filter-list User-Interface agiammarchi
#7239 closed fixed Make links on first-run page more distinct from regular text User-Interface greiner
#7247 closed fixed Improve filter table search visualization User-Interface agiammarchi
#7251 closed fixed Remove currently useless table toggle User-Interface agiammarchi
#7264 closed fixed Update filter table warning message User-Interface agiammarchi
#7288 closed fixed Custom filter table uses removed function User-Interface greiner
#7292 closed fixed Cumulative fixes for io-filter-table User-Interface agiammarchi
#7300 closed fixed Add language dialog mislocated in RTL environment User-Interface agiammarchi
#7304 closed fixed io-filter-list editing long filters is bad UX User-Interface agiammarchi
#7306 closed fixed Line number in filter error refer to wrong filter User-Interface greiner
#7327 closed fixed Update options.showOptions calls to use Promises User-Interface kzar
#7345 closed fixed Use promises for extension API calls in UI (release-2018-5) User-Interface greiner
#7357 closed fixed Disable filter table rich text paste User-Interface agiammarchi
#7439 closed fixed Custom filter table: trigger an error on wrong header User-Interface agiammarchi
#7472 closed fixed Re-set CSS --width on window resize User-Interface agiammarchi


It also imports the following changes that are tracked only on GitLab:

  • ui#281 remove contextual buttons after deleting items
  • ui#303 do not force render the toggle
  • ui#380 Adapt code to work with removed third-party parameter in filter matcher
  • ui#412 Fixed: Weibo link not shown in Help tab
  • ui#415 Unable to add filter lists via abp:subscribe if the options page is closed
  • ui#416 Closing language dropdown issues
  • ui#419 Custom filter table: Disabled filter is re-enabled after refresh in the UI
  • ui#425 Fixed regression: ESLint missing when building for production

It also imports the following Noissue changes:

What to change

  • Change adblockplusui dependency to hg:e1eb6e993ab8 git:8774527.
  • Update the following files:
    • lib/options.js: Return Promise from showOptions()
    • .eslintignore: Remove jquery-ui/js/*
    • metadata.chrome:
      • Add adblockplusui/locale/*/devtools-panel.json = * for translations
      • Update reference from adblockplusui/messageResponder.js to adblockplusui/lib/messageResponder.js
      • Update mappings:
Before After
(none) composer.html = adblockplusui/composer.html
(none) composer.js = adblockplusui/composer.js
(none) skin/composer.css = adblockplusui/skin/composer.css
(none) skin/icons/alert.svg = adblockplusui/skin/icons/alert.svg
(none) skin/icons/copy.svg = adblockplusui/skin/icons/copy.svg
(none) skin/icons/error.svg = adblockplusui/skin/icons/error.svg
icons/detailed/abp-128.png = adblockplusui/skin/abp-128.png icons/detailed/abp-128.png = adblockplusui/skin/icons/abp-128.png
skin/abb-logo.png = adblockplusui/skin/abb-logo.png (none)
skin/abp-logo.svg = adblockplusui/skin/abp-logo.svg skin/icons/abp-logo.svg = adblockplusui/skin/icons/abp-logo.svg
skin/icons/options.svg = adblockplusui/skin/icons/options.svg (none)
skin/social/facebook.svg = adblockplusui/skin/social/facebook.svg skin/icons/facebook.svg = adblockplusui/skin/icons/facebook.svg
skin/social/googleplus.svg = adblockplusui/skin/social/googleplus.svg skin/icons/googleplus.svg = adblockplusui/skin/icons/googleplus.svg
skin/social/twitter.svg = adblockplusui/skin/social/twitter.svg skin/icons/twitter.svg = adblockplusui/skin/icons/twitter.svg
skin/updates/base-graphic.svg = adblockplusui/skin/updates/base-graphic.svg skin/icons/updates/base-graphic.svg = adblockplusui/skin/icons/updates/base-graphic.svg
skin/updates/icon-mobile.svg = adblockplusui/skin/updates/icon-mobile.svg skin/icons/updates/mobile.svg = adblockplusui/skin/icons/updates/mobile.svg
skin/updates/icon-rocket.svg = adblockplusui/skin/updates/icon-rocket.svg skin/icons/updates/rocket.svg = adblockplusui/skin/icons/updates/rocket.svg
skin/updates/icon-thumbs-up.svg = adblockplusui/skin/updates/icon-thumbs-up.svg skin/icons/updates/thumbs-up.svg = adblockplusui/skin/icons/updates/thumbs-up.svg
  • metadata.gecko:
    • Update mappings:
Before After
skin/mobile/checkmark.svg = adblockplusui/skin/mobile/checkmark.svg skin/icons/mobile/checkmark.svg = adblockplusui/skin/icons/mobile/checkmark.svg
skin/mobile/toggle.svg = adblockplusui/skin/mobile/toggle.svg skin/icons/mobile/toggle.svg = adblockplusui/skin/icons/mobile/toggle.svg
skin/mobile/trash.svg = adblockplusui/skin/mobile/trash.svg skin/icons/mobile/trash.svg = adblockplusui/skin/icons/mobile/trash.svg
  • Remove the following files:
    • icons/detailed/abp-32.png
    • jquery-ui/
    • composer.html
    • composer.js

For reference, see changes in UI fork of adblockpluschrome.

Hints for testers

Everything should be implemented according to the spec.

A few of the referenced issues have their own testing instructions:

Furthermore, the following hints apply to the various Noissue changes:

  • Button at the end of the issue reporter should close the issue reporter tab
  • Copyright year should correspond to current year in "About Adblock Plus" dialog in desktop settings page
  • Custom elements should continue to work
    • Enable toggle in bubble UI
    • Screenshot tool in issue reporter
    • Step buttons in header of issue reporter
    • Tooltips in desktop settings page
  • Filter lists in Advanced tab in desktop settings page should be showing corresponding download status
  • Filter lists in Advanced tab in desktop settings page should be sorted alphabetically
  • In mock environment:
    • Data should be initialized in DevTools panel
  • Trash icon should be shown in mobile settings page

Change History (105)

comment:1 Changed 9 months ago by greiner

  • Description modified (diff)

comment:2 Changed 9 months ago by greiner

  • Description modified (diff)

comment:3 Changed 9 months ago by greiner

  • Blocked By 6397 added

comment:4 Changed 9 months ago by greiner

  • Blocked By 5960 added

comment:5 Changed 9 months ago by greiner

  • Blocked By 6880 added

comment:6 Changed 9 months ago by greiner

  • Cc wspee added

comment:7 Changed 9 months ago by greiner

  • Blocked By 6641 added

comment:8 Changed 9 months ago by greiner

  • Blocked By 6960 added

comment:9 Changed 9 months ago by greiner

  • Blocked By 6960 removed

comment:10 Changed 9 months ago by greiner

  • Blocked By 6968 added

comment:11 Changed 9 months ago by greiner

  • Blocked By 6973 added

comment:12 Changed 9 months ago by greiner

  • Blocked By 6323 added

comment:13 Changed 9 months ago by greiner

  • Description modified (diff)

comment:14 Changed 9 months ago by greiner

  • Blocked By 6984 added

comment:15 Changed 8 months ago by greiner

  • Blocked By 7023 added

comment:16 Changed 8 months ago by greiner

  • Blocked By 6765 added

comment:17 Changed 8 months ago by greiner

  • Blocked By 6328 added

comment:18 Changed 8 months ago by greiner

  • Blocked By 7066 added

comment:19 Changed 7 months ago by greiner

  • Blocked By 7106 added

comment:20 Changed 7 months ago by greiner

  • Blocked By 7110 added

comment:21 Changed 7 months ago by greiner

  • Blocked By 7093 added

comment:22 Changed 7 months ago by greiner

  • Blocked By 7093 removed

comment:23 Changed 7 months ago by greiner

  • Blocked By 7110 removed

comment:24 Changed 7 months ago by greiner

  • Blocked By 6324 added

comment:25 Changed 7 months ago by greiner

  • Blocked By 6875 added

comment:26 Changed 7 months ago by greiner

  • Blocked By 6951 added

comment:27 Changed 7 months ago by greiner

  • Blocked By 6952 added

comment:28 Changed 7 months ago by greiner

  • Blocked By 6995 added

comment:29 Changed 7 months ago by greiner

  • Blocked By 7005 added

comment:30 Changed 7 months ago by greiner

  • Blocked By 7022 added

comment:31 Changed 7 months ago by greiner

  • Blocked By 7091 added

comment:32 Changed 7 months ago by greiner

  • Blocked By 7109 added

comment:33 Changed 7 months ago by greiner

  • Blocked By 7118 added

comment:34 Changed 7 months ago by greiner

  • Blocked By 7134 added

comment:35 Changed 7 months ago by greiner

  • Description modified (diff)

Added Noissue changes to ticket description.

comment:36 Changed 7 months ago by greiner

  • Blocked By 7110 added

comment:37 Changed 7 months ago by greiner

  • Description modified (diff)

Filled in empty sections in ticket description.

comment:38 Changed 7 months ago by greiner

  • Blocked By 7127 added

comment:39 Changed 7 months ago by greiner

  • Blocked By 7144 added

comment:40 Changed 5 months ago by greiner

  • Description modified (diff)

Fixed typo in ticket description

comment:41 Changed 3 months ago by greiner

  • Blocked By 7327 added

comment:42 Changed 3 months ago by greiner

  • Blocked By 7140 added

comment:43 Changed 3 months ago by greiner

  • Blocked By 7139 added

comment:44 Changed 3 months ago by greiner

  • Blocked By 7148 added

comment:45 Changed 3 months ago by greiner

  • Blocked By 7229 added

comment:46 Changed 3 months ago by greiner

  • Blocked By 7231 added

comment:47 Changed 3 months ago by greiner

  • Blocked By 7234 added

comment:48 Changed 3 months ago by greiner

  • Blocked By 7239 added

comment:49 Changed 3 months ago by greiner

  • Blocked By 7247 added

comment:50 Changed 3 months ago by greiner

  • Blocked By 7251 added

comment:51 Changed 3 months ago by greiner

  • Blocked By 7264 added

comment:52 Changed 3 months ago by greiner

  • Blocked By 7288 added

comment:53 Changed 3 months ago by greiner

  • Blocked By 7292 added

comment:54 Changed 3 months ago by greiner

  • Blocked By 7300 added

comment:55 Changed 3 months ago by greiner

  • Blocked By 7304 added

comment:56 Changed 3 months ago by greiner

  • Blocked By 7306 added

comment:57 Changed 3 months ago by greiner

  • Blocked By 7228 added

comment:58 Changed 3 months ago by greiner

  • Blocked By 7227 added

comment:59 Changed 3 months ago by greiner

  • Description modified (diff)

Added new Noissue changes to ticket description.

comment:60 Changed 3 months ago by sebastian

  • Cc sebastian added

It seems #7345 also been merged into the 2018-5 release branch, but isn't mentioned in te issue description here, or do I miss something? Any other discrepancies?

comment:61 Changed 3 months ago by greiner

Since Trac isn't being actively kept up-to-date by us, not all tickets have been marked as blocking the dependency update yet but I'd do so as soon as work starts for the next major extension release so that the dependency update can be made ready.

In the meantime, you can check out the GitLab board for this UI release to find out exactly which issues will be part of the dependency update. Alternatively, you'll find all changes that are missing from this ticket in the branch's commit history by looking at the ones created since the last update to this ticket was made.

Hope that helps.

comment:62 Changed 2 months ago by jsonesen

  • Blocking 7308 added

comment:63 Changed 2 months ago by greiner

  • Blocking 7308 removed

comment:64 Changed 5 weeks ago by greiner

  • Blocked By 7357 added

comment:65 Changed 5 weeks ago by greiner

  • Blocked By 7345 added

comment:66 Changed 5 weeks ago by greiner

  • Blocked By 7439 added

comment:67 Changed 5 weeks ago by greiner

  • Blocked By 7472 added

comment:68 Changed 5 weeks ago by greiner

  • Description modified (diff)

Added GitLab-only issues.

comment:69 Changed 5 weeks ago by greiner

  • Description modified (diff)

Updated missing information section.

comment:70 Changed 5 weeks ago by greiner

  • Description modified (diff)

Added specific instructions to what to change section.

comment:71 Changed 5 weeks ago by greiner

  • Blocked By 7106 removed

comment:72 Changed 5 weeks ago by greiner

  • Description modified (diff)

comment:73 Changed 5 weeks ago by greiner

  • Description modified (diff)

comment:74 Changed 5 weeks ago by greiner

  • Description modified (diff)

Added one more GitLab issue that turned out to be necessary for compatibility with adblockpluscore.

comment:75 Changed 5 weeks ago by greiner

  • Description modified (diff)

Added Noissue change that fixes the latest rebase.

comment:76 Changed 4 weeks ago by greiner

  • Description modified (diff)
  • Summary changed from Update adblockplusui dependency to TBD (release-2018-5) to Update adblockplusui dependency to 085509a03c94 (release-2018-5)

Added commit hashes to ticket description.

comment:77 Changed 4 weeks ago by sebastian

  • Priority changed from Unknown to P2
  • Ready set

comment:78 Changed 4 weeks ago by kzar

  • Owner set to kzar

comment:79 Changed 4 weeks ago by kzar

  • Blocked By 7175 added
  • Description modified (diff)

comment:80 Changed 4 weeks ago by kzar

  • Blocked By 5960, 6397, 6765, 6880, 6951, 6952, 6995, 7005, 7022, 7127, 7139, 7140, 7227, 7228 removed

I noticed the following blocking issues which didn't correspond to included commits:

5960, 6397, 6765, 6880, 6951, 6952, 6995, 7005, 7022, 7139, 7140, 7227, 7228

If you'd like to add them back as blocking go ahead, but please make sure they're excluded from the table which lists included changes.

comment:81 Changed 4 weeks ago by kzar

  • Description modified (diff)

comment:82 follow-up: Changed 4 weeks ago by kzar

For the testing hints you mention "the spec", but please could you link to it as well? Also, please could you add testing hints (where relevant) for the code touched by Noissue commits?

comment:83 in reply to: ↑ 82 Changed 4 weeks ago by greiner

  • Description modified (diff)

I noticed the following blocking issues which didn't correspond to included commits:

All of the mentioned tickets (except for the last two which refer to the GitLab issue instead of the Trac ticket) have comments at the end that refer to the commit that got pushed to Mercurial yesterday. Therefore I'm wondering why they wouldn't be included?!

For the testing hints you mention "the spec", but please could you link to it as well? Also, please could you add testing hints (where relevant) for the code touched by Noissue commits?

Sure, I've added a link to the spec repository.

comment:84 Changed 4 weeks ago by kzar

  • Cc rscott Ross philll ukacar added

Heads up, this one will require a lot of testing when it lands!

comment:85 Changed 4 weeks ago by kzar

  • Blocked By 5960, 6397, 6765, 6880, 6951, 6952, 6995, 7005, 7022, 7139, 7140 added
  • Description modified (diff)

Therefore I'm wondering why they wouldn't be included?!

Sorry, shows me for using the GitHub UI instead of the command line... git log 17697d3...bbfb52b --oneline quickly shows me the mising commits. I wish I did that sooner, since git log 17697d3...bbfb52b --oneline | grep -v '[0-9] -' was a lot quicker way of coming up with a list of included Noissue commits as well... Fixed now.

comment:86 Changed 4 weeks ago by kzar

  • Description modified (diff)

comment:87 follow-up: Changed 4 weeks ago by greiner

  • Description modified (diff)

Added hints for testers for Noissue changes.

comment:88 in reply to: ↑ 87 Changed 4 weeks ago by kzar

Replying to greiner:

Added hints for testers for Noissue changes.

Thanks for that.

Please could you go through the included issues and check that they do have suitable testing hints? I went through the first 20 or so issues and didn't find any testing hints at all (including checking the linked GitLab issues). Seems wrong since this issue tells the testers to check out the testing hints in the included issues.

Last edited 4 weeks ago by kzar (previous) (diff)

comment:89 Changed 4 weeks ago by kzar

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

comment:90 Changed 4 weeks ago by kzar

  • Description modified (diff)

As discussed in IRC I've linked to the few issues which do have testing instructions.

comment:91 Changed 4 weeks ago by kzar

  • Description modified (diff)

comment:92 Changed 4 weeks ago by abpbot

A commit referencing this issue has landed:
Issue 6936 - Update adblockplusui dependency to bbfb52b

comment:93 Changed 4 weeks ago by kzar

  • Milestone set to Adblock-Plus-for-Chrome-Opera-Firefox-next
  • Resolution set to fixed
  • Status changed from reviewing to closed

comment:94 Changed 4 weeks ago by kzar

For reference, it seems that the noissue commit Use the io-filter-table instead of the textarea accidentally(?) included a change to package.json which caused eslint to run in production as well. That in turn is causing our CI to fail since the eslint command is not necessarily available. It looks like we'll need to revert that change to package.json and update the adblockplusui dependency again.

Last edited 4 weeks ago by kzar (previous) (diff)

comment:95 Changed 4 weeks ago by greiner

For reference, there's this particular comment that led to this change. We can either revert the change or stop running the linter when creating bundles altogether.

comment:96 Changed 4 weeks ago by kzar

  • Resolution fixed deleted
  • Status changed from closed to reopened

comment:97 Changed 4 weeks ago by kzar

  • Description modified (diff)

comment:98 Changed 4 weeks ago by kzar

  • Description modified (diff)

comment:99 Changed 4 weeks ago by kzar

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

comment:100 Changed 4 weeks ago by abpbot

A commit referencing this issue has landed:
Issue 6936 - Update adblockplusui dependency to 8774527

comment:101 Changed 4 weeks ago by kzar

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

comment:102 Changed 4 weeks ago by kzar

  • Resolution fixed deleted
  • Review URL(s) modified (diff)
  • Status changed from closed to reopened

comment:103 Changed 4 weeks ago by kzar

  • Status changed from reopened to reviewing

comment:104 Changed 4 weeks ago by abpbot

A commit referencing this issue has landed:
Issue 6936 - Adjust Weibo and Google Plus icon mappings

comment:105 Changed 4 weeks ago by kzar

  • Resolution set to fixed
  • Status changed from reviewing to closed
Note: See TracTickets for help on using tickets.