Opened 5 years ago

Closed 3 years ago

#181 closed change (rejected)

Port icon popup from Chrome, Opera and Safari to Firefox

Reported by: greiner Assignee: saroyanm
Priority: P3 Milestone:
Module: User-Interface Keywords:
Cc: fhd, sven, trev, jobp Blocked By: #1435, #1436, #1437, #1438, #1439, #1440, #1445
Blocking: #1461 Platform: Firefox
Ready: no Confidential: no
Tester: Unknown Verified working: no
Review URL(s):

Description (last modified by saroyanm)

Background

In continuation to the ongoing redesign we redid the icon popup in Chrome, Opera and Safari.

What to change

Move the popup's HTML code into Adblock Plus for Firefox, implement the icon popup on there and reuse it in Chrome, Opera and Safari.
List of subtickets:

Ticket Status Resolution Summary Component Owner
#1435 closed rejected [Bubble UI] Port popup.html from Chrome/Safari/Opera to Firefox Adblock-Plus-for-Firefox saroyanm
#1436 closed rejected [Bubble UI] Add Enable/Disable functionality Adblock-Plus-for-Firefox
#1437 closed rejected [Bubble UI] Implement Notifications Adblock-Plus-for-Firefox
#1438 closed rejected [Bubble UI] Add "Statistics" functionality with Ads blocked element Adblock-Plus-for-Firefox
#1439 closed rejected [Bubble UI] Badge implementation Adblock-Plus-for-Firefox
#1440 closed rejected [Bubble UI] Make new Bubble UI compatible with additional ABP Addons. Adblock-Plus-for-Firefox
#1445 closed rejected [Bubble UI] No type attribute in toolbarbutton Adblock-Plus-for-Firefox


After all subtickets are implemented we need to merge bubbleui-firefox repository from bitbucket to adblockplus repository.

Change History (42)

comment:1 Changed 5 years ago by greiner

Further details as provided by Wladimir:

  • replace native Firefox menu with HTML page from Chrome, Opera and Safari embedded into <panel type="arrow">
  • adapt page to account for differing features in the Firefox version (some options can be dropped)
  • necessary functionality from ext.* abstraction layer needs to be added since it doesn't exist in the Firefox version
  • the background page will have to be faked
  • extensions to Adblock Plus (e.g. Diagnostics) need to be added to the popup menu somehow (currently, they're listening for the popupshowing event)

comment:2 Changed 5 years ago by trev

  • Milestone Adblock-Plus-for-Firefox-next deleted
  • Priority changed from P1 to P2

This is planned, no urgency here - P2. Also removed milestone - this is unlikely to make it into the next Firefox release. Currently we only set milestones when an issue is resolved.

comment:3 Changed 5 years ago by greiner

  • Owner set to greiner
  • Status changed from new to assigned

comment:4 Changed 5 years ago by fhd

  • Cc fhd added

comment:5 Changed 5 years ago by philll

  • Status changed from assigned to new

The assigned state will be dropped by #403

comment:6 Changed 5 years ago by fhd

Since this is going to implement the platform abstraction layer we have in Chrome/Opera/Safari for Firefox, can you add the relevant code paths to https://adblockplus.org/en/modules when it's done?

comment:7 Changed 5 years ago by philll

  • Keywords 2014q3 added

comment:8 Changed 5 years ago by jobp

  • Cc jobp@… added

comment:9 Changed 5 years ago by philll

  • Platform set to Firefox

comment:10 Changed 5 years ago by greiner

  • Owner changed from greiner to saroyanm

comment:11 Changed 5 years ago by saroyanm

I've filed a bug on for the remaining issue we have that prevent us from puting the ticket under review, hope we will get response soon from Mozilla developers.
https://bugzilla.mozilla.org/show_bug.cgi?id=1062974

comment:12 Changed 5 years ago by saroyanm

While the bug mentioned above affects MAC OSX, there is another bug that affects both Windows and Linux:
https://bugzilla.mozilla.org/show_bug.cgi?id=1063097

comment:13 Changed 5 years ago by saroyanm

After investigating the Bubble UI further it's came out that the issue is not related to panel of type arrow, but to the toolbarbutton of current types: panel, menu and menu-button.

So the good news are that we can use openPopup method to open the popup and don't use toolbarbutton type attribute.

Also one question would like to align: while the issue affects the toolbarbutton we can still have panel with arrow on the top, but while before we were not using panel with arrow and after aligning with Wladimir I just would like to raise a question whether we want arrow in the panel or not, so what you think guys should we have arrow in the top of popup or stick to the older popup style without arrow ?

comment:14 follow-up: Changed 5 years ago by trev

  • Cc sven added

Also one question would like to align: while the issue affects the toolbarbutton we can still have panel with arrow on the top, but while before we were not using panel with arrow and after aligning with Wladimir I just would like to raise a question whether we want arrow in the panel or not, so what you think guys should we have arrow in the top of popup or stick to the older popup style without arrow ?

Sven, any opinion? Firefox convention is that toolbar buttons with a dropdown of some kind (which would include our bubble) should have a dropdown arrow. However, Firefox currently violates this convention itself with the bookmark icon (granted, it only shows a bubble on the second click). Also, our icon doesn't have this dropdown arrow in Chrome or Safari. Essentially, it is platform consistency vs. product consistency.

comment:15 Changed 5 years ago by trev

  • Cc trev jobp added; jobp@… removed

comment:16 in reply to: ↑ 14 ; follow-up: Changed 5 years ago by saroyanm

Replying to trev:

Sven, any opinion? Firefox convention is that toolbar buttons with a dropdown of some kind (which would include our bubble) should have a dropdown arrow. However, Firefox currently violates this convention itself with the bookmark icon (granted, it only shows a bubble on the second click). Also, our icon doesn't have this dropdown arrow in Chrome or Safari. Essentially, it is platform consistency vs. product consistency.

Wladimir, so just for clarification as far as I understand you mean the dropdown arrow which is on the right side of toolbarbutton icon, not the arrow that is specified by <panel type="arrow"> ?
Maybe in that case we should also think about the case of changing the default toolbar action, I think in case of left click action change using ABP customization maybe we will still need that dropdown there.

Last edited 5 years ago by saroyanm (previous) (diff)

comment:17 in reply to: ↑ 16 Changed 5 years ago by sven

Replying to saroyanm:

Replying to trev:

Sven, any opinion? Firefox convention is that toolbar buttons with a dropdown of some kind (which would include our bubble) should have a dropdown arrow. However, Firefox currently violates this convention itself with the bookmark icon (granted, it only shows a bubble on the second click). Also, our icon doesn't have this dropdown arrow in Chrome or Safari. Essentially, it is platform consistency vs. product consistency.

Wladimir, so just for clarification as far as I understand you mean the dropdown arrow which is on the right side of toolbarbutton icon, not the arrow that is specified by <panel type="arrow"> ?
Maybe in that case we should also think about the case of changing the default toolbar action, I think in case of left click action change using ABP customization maybe we will still need that dropdown there.

I see both variants in firefox extensions. Ghostery for example has no arrow but Greasemonkey has. I think that it makes most sense if we implement it on every platform in the same way. So the conclusion would be: no arrow down for firefox anymore since we have no arrow at other platforms.

comment:18 Changed 5 years ago by saroyanm

Guys I have a question about Badge implementation in Seamonkey,
The badged type for toolbarbutton in Seamonkey looks not to implemented, but in Firefox we can use the badge freely, so I would like to align if we really need the badge for showing hit statistics on active page in Seamonkey or not ?
In case we need badge also in Seamonkey seams to be we need to implement the badge ourself and don't use Firefox implementation for it.

comment:19 Changed 5 years ago by saroyanm

After discussing today with Wladimir the Bubble UI implementation looks like we'll need to implement the badge ourselves and don't use Firefox implementation because it's not documented and the implementation looks more to be for internal use.

Also we will need to split the patch and create several subtickets to make the review process not be much complicated with a one huge patch.

comment:20 Changed 5 years ago by saroyanm

  • Blocked By 1435 added

comment:21 Changed 5 years ago by saroyanm

  • Blocked By 1436 added

comment:22 Changed 5 years ago by saroyanm

  • Blocked By 1437 added

comment:23 Changed 5 years ago by saroyanm

  • Blocked By 1438 added

comment:24 Changed 5 years ago by saroyanm

  • Blocked By 1439 added

comment:25 Changed 5 years ago by saroyanm

  • Blocked By 1440 added

comment:26 Changed 5 years ago by saroyanm

  • Description modified (diff)

comment:27 Changed 5 years ago by saroyanm

  • Blocked By 1445 added

comment:28 Changed 5 years ago by saroyanm

There is a new repository for this issue in bitbucket where we are going to push changes before merging with adblockplus repo.

comment:29 follow-up: Changed 5 years ago by greiner

Keep in mind that we also need to backport these changes to Platform and update the subrepository.

comment:30 Changed 5 years ago by saroyanm

  • Blocking 1461 added

comment:31 Changed 5 years ago by saroyanm

  • Description modified (diff)

comment:32 Changed 5 years ago by saroyanm

  • Description modified (diff)

comment:33 in reply to: ↑ 29 Changed 5 years ago by saroyanm

Replying to greiner:

Keep in mind that we also need to backport these changes to Platform and update the subrepository.

Yes I've added a new Blocking ticket #1461.

comment:34 Changed 5 years ago by fhd

  • Keywords 2014q4 added; 2014q3 removed

comment:35 Changed 5 years ago by jobp

Any chance we can get this with a slightly higher priority for the upcoming quarter? This project has been laying around for over 9 months, and we are missing out that 17MM daily active users are unable to share ABP through the bubble UI.

Would be awesome if we can squeeze this in.

Last edited 5 years ago by jobp (previous) (diff)

comment:36 follow-up: Changed 5 years ago by philll

This issue is part of the current roadmap and thus already the highest non-bug or business-endangering priority. If there wasn't too much progress here, the reason is for sure not the priority.

comment:37 in reply to: ↑ 36 ; follow-up: Changed 5 years ago by jobp

Replying to philll:

If there wasn't too much progress here, the reason is for sure not the priority.

Fair enough.

Then it looks like nobody really picked it up - even though the P2 was set 9 months ago.
@Thomas: Since you are the module owner of our UIs, mind helping to push for this?

comment:38 in reply to: ↑ 37 Changed 5 years ago by saroyanm

Replying to jobp:

Then it looks like nobody really picked it up - even though the P2 was set 9 months ago.

Subticket is pending a review, so in case you would like to increase priority, or push the ticket, make sense to do so with subticket, in this case #1435, I think it's the biggest subticket, so other ones should go much more quicker.

comment:39 Changed 5 years ago by jobp

Ok, thanks.

comment:40 Changed 4 years ago by greiner

  • Ready unset

Since we now have the adblockplusui repository for shared UI components it makes more sense to move it there and then make use of it in Adblock Plus for Firefox. It should also make use of the recently introduced front-end messaging API to ensure compatibility with e10s.

For that we need to adapt all of the existing issues and extend and clarify their descriptions before we can start working on those again.

comment:41 Changed 4 years ago by fhd

  • Keywords 2014q4 removed
  • Priority changed from P2 to P3
  • Tester set to Unknown

This doesn't seem to qualify as a P2 anymore, changing to P3.

comment:42 Changed 3 years ago by sebastian

  • Resolution set to rejected
  • Status changed from new to closed

With traditional Firefox extensions to be deprecated, and Adblock Plus for Firefox to be replaced with something that is build directly from adblockpluschrome the effort here becomes obsolete.

Note: See TracTickets for help on using tickets.