Opened 14 months ago

Closed 14 months ago

Last modified 14 months ago

#4510 closed defect (fixed)

Preferences: Findbar not completely functional in Firefox 52.0a1

Reported by: trev Assignee: trev
Priority: P2 Milestone: Adblock-Plus-2.8-for-Firefox
Module: Adblock-Plus-for-Firefox Keywords:
Cc: Blocked By:
Blocking: Platform: Unknown / Cross platform
Ready: yes Confidential: no
Tester: Ross Verified working: yes
Review URL(s):

https://codereview.adblockplus.org/29356477/

Description

Environment

Adblock Plus 2.7.3.4207
Firefox 52.0a1 nightly
Mac OS X 10.12

How to reproduce

  1. Open Filter Preferences.
  2. Click Find button.
  3. Type "asdf".

Observed behaviour

Findbar opens but isn't focused so the text you typed goes nowhere. After clicking the text field you can type in the search text and it appears to work. Error Console shows the following exceptions:

TypeError: this.browser.finder.onFindbarOpen is not a function findbar.xml:738:13
TypeError: this.browser.finder.onFindbarClose is not a function findbar.xml:762:11
TypeError: this.browser.finder.destroy is not a function findbar.xml:427:13

Expected behaviour

The findbar should be focused when opening and there should be no exceptions.

What to change

The findbar widget once again expects new APIs that we don't provide. We need to stop using it and create a look-alike instead.

Attachments (1)

findbar.png (26.6 KB) - added by trev 14 months ago.
Screenshot of the current state

Download all attachments as: .zip

Change History (5)

Changed 14 months ago by trev

Screenshot of the current state

comment:1 Changed 14 months ago by trev

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

Here is what it looks right now:

Screenshot of the current state

I placed it under the filters list since that's what is being searched, it no longer spans the entire width of the dialog. And it's not identical to the "real" findbar of course, there are quite a few differences:

  • It's a real search field, so pressing Esc will reset the text. Only pressing Esc again will close the search.
  • Firefox is missing styles for checked buttons on macOS. So for the pressed "Match Case" button we have to fake it by applying brightness(70%) filter to the button when it is pressed.
  • We cannot set red background on a search field (due to -moz-appearance), so I implemented a red shadow instead.
  • There was no good way to implement native-looking next/previous buttons so I used a <spinbuttons> element. It's quite compact and the buttons are placed on top of each other which makes sense logically, the buttons are rather hard to hit with the mouse however.
  • As the next/previous buttons cannot be selected via keyboard, Up/Down keys can be used to switch to the next/previous results. Page Up and Page Down can be used to scroll the list as expected.
  • There is no Find As You Type functionality - you have to explicitly press Ctrl/Cmd-F or click "Find" to start searching. I didn't implement this because of significant complexity, but there is also the problem that this functionality would have to consider findbar preferences - that would again rely on Firefox internals.

comment:3 Changed 14 months ago by trev

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

comment:4 Changed 14 months ago by Ross

  • Tester changed from Unknown to Ross
  • Verified working set

Done. Findbar appears to be working correctly on OS X and works as described. Agree the spin buttons are slightly hard to hit with the mouse.

ABP 2.7.3.4214-beta
Firefox 38 / 46 / 49 / Windows 7
Firefox Nightly 52.0a1 (2016-10-20) / OS X 10.12

Note: See TracTickets for help on using tickets.