Closed Bug 1357800 Opened 7 years ago Closed 7 years ago

The One-Off search buttons not visible in the search bar

Categories

(Firefox :: Search, defect, P1)

55 Branch
All
Linux
defect

Tracking

()

VERIFIED FIXED
Firefox 55
Tracking Status
firefox-esr52 --- unaffected
firefox53 --- unaffected
firefox54 --- unaffected
firefox55 --- verified
firefox57 --- verified

People

(Reporter: sbadau, Assigned: adw)

References

Details

(Keywords: regression, Whiteboard: [fxsearch])

Attachments

(2 files)

Attached image OneOffsButtons.png
[Affected versions]:
- Nightly 55.0a1 - Build ID: 20170419100228

[Affected platforms]:
- Linux

[Steps to reproduce]:
1. Launch Firefox with a new profile
2. Open the Add-ons Manager
3. Enable the Compact Dark team
4. Type something in the Search Bar (look at the one-off search buttons)
5. Enable the Default team 
6. Double click in the Search Bar (look at the one-offs search buttons)

[Expected result]:
- In step 6, the second row of the one-off search buttons is properly displayed

[Actual result]:
- In step 6, the second row of the one-off search buttons is white. Hovering the mouse over the second row repaints it. Please see the screenshot for more details.
Strange...
Assignee: nobody → adw
Blocks: 1295460
No longer blocks: 1180944
Status: NEW → ASSIGNED
Priority: -- → P1
Whiteboard: [fxsearch]
This sounds like a regression from bug 1312999 rather than bug 1295460.
Blocks: 1312999
No longer blocks: 1295460
Keywords: regression
Seems to be a graphics bug.
(In reply to Florian Quèze [:florian] [:flo] from comment #3)
> Seems to be a graphics bug.

What makes you think so?
(In reply to Dão Gottwald [::dao] from comment #4)
> (In reply to Florian Quèze [:florian] [:flo] from comment #3)
> > Seems to be a graphics bug.
> 
> What makes you think so?

That it happens only on Linux, and that the problem fixes itself when hovering a one-off button from the first line (we don't run any relevant JS code when doing that; the hover effect is a CSS :hover).
If we want to workaround, we could just add another observer near http://searchfox.org/mozilla-central/source/browser/components/search/content/search.xml#1422 to reset the one-off buttons when the theme is changed.
From what I've seen the JS implementing the one-off search button grid hasn't been all too reliable from the start, so I'd bet it's a problem there.

Do we change anything on hover that might affect layout, like margins or borders?
(In reply to Dão Gottwald [::dao] from comment #7)

> Do we change anything on hover that might affect layout, like margins or
> borders?

I don't think so, http://searchfox.org/mozilla-central/rev/d8ac097a1de2544f0e51d186bc850662c5b7430a/browser/themes/linux/searchbar.css#225
I can't reproduce this in my Ubuntu VM.  Simona, are you using hardware acceleration?  I agree with Florian that this looks like a graphics problem.  When I open about:support, under the Graphics section, it says:

> Features > Compositing > Basic
> Decision Log > HW_COMPOSITING > blocked by default: Acceleration blocked by platform
> Decision Log > OPENGL_COMPOSITING > unavailable by default: Hardware compositing is disabled

... which I guess means I'm not using hardware acceleration.

Would it be possible for you to find a regression range?
Flags: needinfo?(simona.marcu)
Mozilla/5.0 (X11; Linux x86_64; rv:55.0) Gecko/20100101 Firefox/55.0
Build ID: 20170424100313

Regression range: 
Last good revision: 13fd2a40e7d689b3b4582759972ccdc32e203b68
First bad revision: 1735bad11778eee56faadccb916c31b9ad9e0be5
Pushlog:
https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=13fd2a40e7d689b3b4582759972ccdc32e203b68&tochange=1735bad11778eee56faadccb916c31b9ad9e0be5

I would say that I'm also not using Hardware acceleration giving the Graphics section from about:support.
Decision Log
HW_COMPOSITING	
blocked by default: Acceleration blocked by platform
OPENGL_COMPOSITING	
unavailable by default: Hardware compositing is disabled
WEBRENDER	
opt-in by default: WebRender is an opt-in feature

I haven't tried to reproduce this issue on a VM machine but I managed to reproduce it on 3 different physical Ubuntu 16.04 machines.
Flags: needinfo?(simona.marcu)
(In reply to Simona B [:simonab ] from comment #10)
> Mozilla/5.0 (X11; Linux x86_64; rv:55.0) Gecko/20100101 Firefox/55.0
> Build ID: 20170424100313
> 
> Regression range: 
> Last good revision: 13fd2a40e7d689b3b4582759972ccdc32e203b68
> First bad revision: 1735bad11778eee56faadccb916c31b9ad9e0be5
> Pushlog:
> https://hg.mozilla.org/integration/mozilla-inbound/
> pushloghtml?fromchange=13fd2a40e7d689b3b4582759972ccdc32e203b68&tochange=1735
> bad11778eee56faadccb916c31b9ad9e0be5

This confirms bug 1312999 regressed this.
Flags: needinfo?(florian)
I still think it was a graphics bugs that was hidden by the excessive rebuilding of the one-off buttons before that fix. Do you know who works on fixing Linux graphics bugs? If we need to add a workaround, I proposed one in comment 6, but it would be nice to at least try to have someone look at the actual bug.
Flags: needinfo?(florian)
This is not about the one-off buttons in the awesomebar.
No longer blocks: 1337003
(In reply to Florian Quèze [:florian] [:flo] from comment #12)
> I still think it was a graphics bugs that was hidden by the excessive
> rebuilding of the one-off buttons before that fix. Do you know who works on
> fixing Linux graphics bugs? If we need to add a workaround, I proposed one
> in comment 6, but it would be nice to at least try to have someone look at
> the actual bug.

Milan, can someone confirm whether this is a graphics bug?
Flags: needinfo?(milan)
I guess anything is possible :)  I don't really have anybody that'd look into this in the short term.  If we know of a workaround, I'd suggest we use it.
Flags: needinfo?(milan)
Panos, do we know if this is work-around-able?
Flags: needinfo?(past)
We have a suggested workaround in comment 6, just nobody in my team with free cycles yet.
Flags: needinfo?(past)
Marking as fix-optional for 55, but if someone wants to get it fixed via comment 6, that's cool too.
Simona, could you please try this build (or the patch I just posted)?  It does what Florian suggests in comment 6: rebuild the buttons the next time the popup is opened after a theme change.  It'll be interesting to see whether that fixes the problem.

https://archive.mozilla.org/pub/firefox/try-builds/dwillcoxon@mozilla.com-95c1fc8ce7b449d8dada8c20b46004aad6b1aad1/
Flags: needinfo?(simona.marcu)
(In reply to Drew Willcoxon :adw from comment #21)
> Simona, could you please try this build (or the patch I just posted)?  It
> does what Florian suggests in comment 6: rebuild the buttons the next time
> the popup is opened after a theme change.  It'll be interesting to see
> whether that fixes the problem.
> 
> https://archive.mozilla.org/pub/firefox/try-builds/dwillcoxon@mozilla.com-
> 95c1fc8ce7b449d8dada8c20b46004aad6b1aad1/

Drew, on the above link I'm only seeing builds for Windows and Mac OS X. Could you please help me with an Ubuntu try-build (I could only reproduce this issue on Ubuntu)?
Flags: needinfo?(simona.marcu) → needinfo?(adw)
(In reply to Simona B [:simonab ] from comment #22)
> (In reply to Drew Willcoxon :adw from comment #21)
> > Simona, could you please try this build (or the patch I just posted)?  It
> > does what Florian suggests in comment 6: rebuild the buttons the next time
> > the popup is opened after a theme change.  It'll be interesting to see
> > whether that fixes the problem.
> > 
> > https://archive.mozilla.org/pub/firefox/try-builds/dwillcoxon@mozilla.com-
> > 95c1fc8ce7b449d8dada8c20b46004aad6b1aad1/
> 
> Drew, on the above link I'm only seeing builds for Windows and Mac OS X.
> Could you please help me with an Ubuntu try-build (I could only reproduce
> this issue on Ubuntu)?

I think this is a link to the Linux64 PGO build: https://queue.taskcluster.net/v1/task/APG7KvzZQaG1bY2GyArnhg/runs/0/artifacts/public/build/target.tar.bz2
(In reply to Florian Quèze [:florian] [:flo] from comment #23)
> (In reply to Simona B [:simonab ] from comment #22)
> > (In reply to Drew Willcoxon :adw from comment #21)
> > > Simona, could you please try this build (or the patch I just posted)?  It
> > > does what Florian suggests in comment 6: rebuild the buttons the next time
> > > the popup is opened after a theme change.  It'll be interesting to see
> > > whether that fixes the problem.
> > > 
> > > https://archive.mozilla.org/pub/firefox/try-builds/dwillcoxon@mozilla.com-
> > > 95c1fc8ce7b449d8dada8c20b46004aad6b1aad1/
> > 
> > Drew, on the above link I'm only seeing builds for Windows and Mac OS X.
> > Could you please help me with an Ubuntu try-build (I could only reproduce
> > this issue on Ubuntu)?
> 
> I think this is a link to the Linux64 PGO build:
> https://queue.taskcluster.net/v1/task/APG7KvzZQaG1bY2GyArnhg/runs/0/
> artifacts/public/build/target.tar.bz2

Thank you, Florian. 

I can no longer reproduce the initial issue when using the provided try build on Ubuntu 16.04 x64.
Flags: needinfo?(adw)
That's strange, I asked try to build on Linux too.  Anyway, thanks Florian, glad you found a build and couldn't reproduce the problem anymore, Simona.
Attachment #8867972 - Flags: review?(florian)
(In reply to Drew Willcoxon :adw from comment #25)
> That's strange, I asked try to build on Linux too.

Your try push did build Linux, it just seems the task cluster builds don't get uploaded to archive.mozilla.org.
Comment on attachment 8867972 [details]
Bug 1357800 - The One-Off search buttons not visible in the search bar.

https://reviewboard.mozilla.org/r/139506/#review143614

Thanks for taking care of this bug!

::: commit-message-0f4df:1
(Diff revision 1)
> +Bug 1357800 - The One-Off search buttons not visible in the search bar.

Please make the commit message describe a bit better what this is doing and why.

::: browser/components/search/content/search.xml:1422
(Diff revision 1)
>          });
>  
>          // Add weak referenced observers to invalidate our cached list of engines.
>          Services.prefs.addObserver("browser.search.hiddenOneOffs", this, true);
>          Services.obs.addObserver(this, "browser-search-engine-modified", true);
> +        Services.obs.addObserver(this, "lightweight-theme-changed", true);

This code change is fine, but I think we should add a comment above it mentioning that this is a workaround for what we believe to be a Linux-only graphics bug (and maybe mention the bug number). Otherwise someone may remove this by accident... or we may forget to remove it once it's no longer relevant (if the graphics bug happens to disappear on its own a year or two from now).
Attachment #8867972 - Flags: review?(florian) → review+
Fixed, thanks
Pushed by dwillcoxon@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/30da70a81554
The One-Off search buttons not visible in the search bar. r=florian
https://hg.mozilla.org/mozilla-central/rev/30da70a81554
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 55
See Also: → 1397248
Indirectly verified this as part of Photon bug verification on:
   Ubuntu 16.04, Windows 10 and Mac OsX 10.12 (although the bug was originally Linux only, I thought it safer to check it on the other OS'es as well due to bug 1397248)
                        - 57.0a1 20170905220108
                        - 55.0.3 20170824053622
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: