1491526 - Port bug Bug 1411707: Findbar buttons have no labels

Status: RESOLVED FIXED

(Thunderbird :: General, task)

Description

[Richard Marti (:Paenglab)]

Since bug 1411707 the findbar buttons have no labels. It seems findbar.ftl isn't found. It is on the correct position in the build like the other ftl files.

Comment 1

[Richard Marti (:Paenglab)]

Brian, do you know what the issue could be that TB doesn't load the findbar.ftl? We use already FTL files, so this is no global problem with this type of files. In the console I see no errors.

Comment 2

[Brian Grinstead [:bgrins]]

Do you have the findbar in the markup, or does it get created in DOMContentLoaded? If so I think this might be a limitation of the current solution for dynamically detected FTL links added in Bug 1490457. We'll have to fix that anyway for future CE, will try and take a look at that next week.

In the meantime, could you try adding it in the markup first thing after the <window> and let me know if it fixes it?

  <linkset>
    <link rel="localization" href="toolkit/main-window/findbar.ftl"/>
  </linkset>

Comment 3

[Richard Marti (:Paenglab)]

Adding the <linkset> fixes it.

Comment 4

[Brian Grinstead [:bgrins]]

(In reply to Richard Marti (:Paenglab) from comment #3)
> Adding the <linkset> fixes it.

OK, I'd probably do that in the meantime and then drop it post Bug 1491549. Note that once you have the first link during parse, new links (for future CE migrations) will be properly detected even without that bug fix.

Comment 5

[Richard Marti (:Paenglab)]

Okay, I'll do this. Many thanks!

Findbar should be in the markup, see https://searchfox.org/comm-central/search?q=findbar&case=false&regexp=false&path=mail%2Fbase

Comment 6

[Richard Marti (:Paenglab)]

Attachment: findbar-link.patch

This fixes the missing labels.

Brian was so nice to cc me on bug 1491549. Then I know when I can try to revert this again.

Comment 7

[Brian Grinstead [:bgrins]]

Besides this issue, is the findbar working as expected without changes on your side after the Custom Element migration?

Comment 8

[Richard Marti (:Paenglab)]

I think everything works except the "Highlight All". And it does also not show how many times it found the search term. By going forward/backward it shows the next/previous match.

Comment 9

[Jorg K (CEST = GMT+2)]

I think we should address all malfunctions here.

Comment 10

[Jorg K (CEST = GMT+2)]

Comment on attachment 9009358 [details] [diff] [review]
findbar-link.patch

Thanks, this is OK, but we'll need another patch to restore the functionality completely.

Comment 11

[Richard Marti (:Paenglab)]

OK Jörg, please cold you look into this? Or who could look?

Comment 12

[Jorg K (CEST = GMT+2)]

(In reply to Richard Marti (:Paenglab) from comment #11)
> OK Jörg, please cold you look into this? Or who could look?
I looked, highlight and counting work on the main window, I see no problem. I've counted "e"s on the page. However, they don't work on the stand-alone and compose windows.

Are we missing the injection of some JS there or something similar?

Comment 13

[Jorg K (CEST = GMT+2)]

Brian, any hint what we need to inject? I remember porting <editor> to a custom element (bug 1478139) and there we didn't have to do anything.

Or maybe Magnus knows, he's been working on de-XBL as well.

Comment 14

[Richard Marti (:Paenglab)]

(In reply to Jorg K (GMT+2) from comment #12)
> (In reply to Richard Marti (:Paenglab) from comment #11)
> > OK Jörg, please cold you look into this? Or who could look?
> I looked, highlight and counting work on the main window, I see no problem.
> I've counted "e"s on the page.

Hmm, strange, on Daily without the labels it works. On my own build not.

Comment 15

[Jorg K (CEST = GMT+2)]

I tested this again. The highlight works on all windows. It's just flaky like it always was. Sometimes you have to click "forward" for the highlight to appear.

I think the patch is all we need, sorry about the noise.

Comment 16

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/d01e5283a6d7
Port bug 1411707: Add the findbar.ftl link to the windows using the findbar. r=jorgk

Comment 17

[Jorg K (CEST = GMT+2)]

Let's assume we're done here for now. Richard, can you please test a Daily.

Comment 18

[Jorg K (CEST = GMT+2)]

Tested this now.

(In reply to Jorg K (GMT+2) from comment #15)
> I tested this again. The highlight works on all windows. It's just flaky
> like it always was. Sometimes you have to click "forward" for the highlight
> to appear.
On the main window, cancelling highlight and highlighting again works. On the stand-alone and compose windows, highlighting again only works when clicking "forward". Strange but true. But nothing we can address here, I think.