Closed Bug 1624238 Opened 5 months ago Closed 3 months ago

browserAction.setTitle() has problems with newline characters

Categories

(WebExtensions :: Untriaged, defect, P2)

defect

Tracking

(firefox77 fixed)

RESOLVED FIXED
mozilla77
Tracking Status
firefox77 --- fixed

People

(Reporter: kernp25, Assigned: nhnt11)

References

(Regressed 1 open bug)

Details

Attachments

(10 files, 3 obsolete files)

Attached file test.zip (obsolete) —

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:74.0) Gecko/20100101 Firefox/74.0

Steps to reproduce:

  1. Load the attached add-on

Actual results:

It will show: "First line.Second line."

See Image.

Expected results:

It should just show: "First line."

But the tooltip should still display:
First line.
Second line.

Attached image without-tooltip.png (obsolete) —
Attached image with-tooltip.png (obsolete) —

What do you think?

The use case is that, in my other add-on i want to display important information in the tooltip.

But it looks bad when the button is in the popup panel (moved by user).

Flags: needinfo?(rob)

Expected results:

In the popup panel:
My add-on

In the tooltip:
My add-on
Important information

Or other option:
The popup panel should just show the add-on name.
And the tooltip should display what the add-on set with browserAction.setTitle().

Results would be:

In the popup panel:
My add-on

In the tooltip:
Important information

Attached file test.zip
Attachment #9134994 - Attachment is obsolete: true
Attached image image_1.png
Attachment #9134995 - Attachment is obsolete: true
Attachment #9134996 - Attachment is obsolete: true
Attached image image_2.png
Attached image image_3.png

How it looks like in the popup window.

The bug report is unclear. Could you explain what you're expecting and what happens instead, for a given STR?

Flags: needinfo?(rob)
Flags: needinfo?(kernp25)

Tooltips does support line breaks but when using it with browserAction.setTitle() it will look strange in the menu panel.

See image from: https://bugzilla.mozilla.org/show_bug.cgi?id=1624238#c9

Flags: needinfo?(kernp25)
Attached image RjY9k5HGdj.png

How it looks like with only 1 line.

The bug is that, the menu panel is broken when the title contains line breaks.

Expected results:
browserAction.setTitle() should support line breaks, but without breaking the menu panel.

When I load the extension, reveal the add-on button and click on it to open the popup, I see that the panel is loaded and that the heading is shifted to the left. This looks wrong. Are you referring to this bug?

I tried to debug this with the Browser Toolbox, and disabling popup auto-hide. After inspecting the affected area, I see the following markup of interest:

<panelview ...
  id="PanelUI-webext-adf8405353d0cd89d86893ce618ef0f0acf90f88_temporary-addon-browser-action-view" ...
  style="max-width: 348px; min-width: 348px;" ...
>
  <box class="panel-header">
    <toolbarbutton ...>... </toolbarbutton>
    <label>
      <span xmlns="http://www.w3.org/1999/xhtml">
Firstttttttttttttttt line.
Secondddddddddddddd line.
Enddddddddddddd line.
      </span>
    </label>
  </box>
  ...
</panelview>

The <panelview> element's dimensions is constrained by the max-width property, which appears to be set here: https://searchfox.org/mozilla-central/rev/b712398b7fae54ef377a558d6f16dede7a7f8530/browser/components/customizableui/PanelMultiView.jsm#718

When the inline style is not set, it falls back to the max-width (=29em) from the stylesheet at https://searchfox.org/mozilla-central/rev/b712398b7fae54ef377a558d6f16dede7a7f8530/browser/themes/shared/customizableui/panelUI.inc.css#374-375

The visual bug disappears when I remove overflow:hidden from the panelview - https://searchfox.org/mozilla-central/rev/b712398b7fae54ef377a558d6f16dede7a7f8530/browser/themes/shared/customizableui/panelUI.inc.css#349-351

I don't know why though...

Status: UNCONFIRMED → NEW
Ever confirmed: true

(In reply to Rob Wu [:robwu] from comment #15)

When I load the extension, reveal the add-on button and click on it to open the popup, I see that the panel is loaded and that the heading is shifted to the left. This looks wrong. Are you referring to this bug?

I only tested it in Firefox 74.0. But it must be the same bug, as mentioned in Comment 13.

Gijs, see comment 15. Do you have an idea what's going on?

I'm also wondering why removing overflow:hidden fixes the bug (it was introduced in bug 1282189).
(I haven't checked for side effects, i.e. the impact of its removal in other cases.)

Flags: needinfo?(gijskruitbosch+bugs)

What do you think of the idea from Comment 5?

Flags: needinfo?(rob)

I'm not against showing the add-on name instead of the tooltip, but it seems that the heading is derived from the title: https://searchfox.org/mozilla-central/rev/b712398b7fae54ef377a558d6f16dede7a7f8530/browser/components/customizableui/PanelMultiView.jsm#714

The implementation can change if really needed, but I'd like to avoid too much special-casing if possible.

Flags: needinfo?(rob)

(In reply to Rob Wu [:robwu] from comment #19)

I'm not against showing the add-on name instead of the tooltip, but it seems that the heading is derived from the title: https://searchfox.org/mozilla-central/rev/b712398b7fae54ef377a558d6f16dede7a7f8530/browser/components/customizableui/PanelMultiView.jsm#714

The implementation can change if really needed, but I'd like to avoid too much special-casing if possible.

But you can see, how bad it looks like in the image from Comment 7.

My add-on does this:
test@gmail : 5
test2@yahoo.com : 10

It is a email notifier and it would like to display the email address with the email count.

It would display in the menu panel like this:
test@gmail : 5test2@yahoo.com : 10

Attached image IMw87Bkixx.png
Attached image 9w3PCp83bc.png

How it looks like in Google Chrome.

(In reply to Rob Wu [:robwu] from comment #17)

Gijs, see comment 15. Do you have an idea what's going on?

Not off-hand. It looks vaguely like something that's supposed to be in a subtree under an element with overflow:hidden and thus invisible (having been "slid out of view" after a subview animation is not, in fact, invisible. I don't know why that would be. Equally, the top bit looks cut-off mid-item, which almost looks like a graphics issue of some kind - unless the text is forcing the element to be wider than its container, and the overflow:hidden means the left side gets cut off or something?

I'm also wondering why removing overflow:hidden fixes the bug (it was introduced in bug 1282189).
(I haven't checked for side effects, i.e. the impact of its removal in other cases.)

Generally, I'm confused by the direction this bug is going in. It seems the request in comment #0 was relatively simple: make the title a single-line string (so strip anything after the first newline, if any), and display the full thing in the tooltip. It seems like we could do that and that'd also fix the display issue you're seeing? Is there some reason we don't want to pursue that?

Flags: needinfo?(gijskruitbosch+bugs) → needinfo?(rob)

(In reply to :Gijs (he/him) from comment #24)

Equally, the top bit looks cut-off mid-item, which almost looks like a graphics issue of some kind - unless the text is forcing the element to be wider than its container, and the overflow:hidden means the left side gets cut off or something?

Which top? In the video in comment 15, the top-right part is a rendering issue (demonstrated by the inability to interact with it by the mouse). Often it is just blank.
The line that ends midway is normal, it is the border-bottom of the .panel-header element.

I'm also wondering why removing overflow:hidden fixes the bug (it was introduced in bug 1282189).
(I haven't checked for side effects, i.e. the impact of its removal in other cases.)

Generally, I'm confused by the direction this bug is going in. It seems the request in comment #0 was relatively simple: make the title a single-line string (so strip anything after the first newline, if any), and display the full thing in the tooltip.

The bug was not clear to me and it's a moving target (see e.g. comment 5). I'm more concerned about the rendering bug though, and am looking into it to see if we can file a follow-up bug to fix whatever underlying bug is there.
The rendering bug is not limited to line breaks; I can reproduce the same issue if I load an extension with a very long title:

{
    "name": "Long button title",
    "version": "1",
    "manifest_version": 2,
    "browser_action": {
        "default_area": "menupanel",
        "default_popup": "manifest.json",
        "default_title": "abcdefghijklmnopqrstuvwxyz0123456789_abcdefghijklmnopqrstuvwxyz0123456789_abcdefghijklmnopqrstuvwxyz0123456789"
    }
}

It seems like we could do that and that'd also fix the display issue you're seeing? Is there some reason we don't want to pursue that?

I tried white-space:nowrap, and although that cut the title, the rendering issue (=a big gap appearing at the right) did not disappear.

If we can't figure out the bug here, I can just file a new bug (in which product/component?) and let the triage owner handle it.
Then this bug can return to kernp's request (which is either in comment 0 or comment 5).

Flags: needinfo?(rob)

See previous comment

Flags: needinfo?(gijskruitbosch+bugs)

(In reply to Rob Wu [:robwu] from comment #25)

If we can't figure out the bug here, I can just file a new bug (in which product/component?) and let the triage owner handle it.
Then this bug can return to kernp's request (which is either in comment 0 or comment 5).

That seems like a good idea. I take it just text-overflow: ellipsis on the title doesn't fix it? If not, file a new bug in Firefox :: Toolbars & Customization and ping me there, please. Thank you!

Flags: needinfo?(gijskruitbosch+bugs) → needinfo?(rob)
See Also: → 1625600

I added text-overflow:ellipsis on every element from the <span> element and upwards, but didn't see any difference in behavior.

I've filed bug 1625600 and included a bit of interaction with the devtools in the video, please take a look when you have some time.

Flags: needinfo?(rob)

But i think, if you fix bug 1625600, it would also work for me, if you replace the line with a space. Then it would look like this:
test@gmail : 5 test2@yahoo.com : 10

And not:
test@gmail : 5test2@yahoo.com : 10

The priority flag is not set for this bug.
:mixedpuppy, could you have a look please?

For more information, please visit auto_nag documentation.

Flags: needinfo?(mixedpuppy)
Flags: needinfo?(mixedpuppy)

The priority flag is not set for this bug.
:mixedpuppy, could you have a look please?

For more information, please visit auto_nag documentation.

Flags: needinfo?(mixedpuppy)

FWIW, I think bug 1574196 made the header wrap in the protections/identity panel cases... but only added styling for those controls, while adding a <span> (and moving away from label.value) everywhere.

Nihanth, can you check if that caused a change for webextensions? I would expect so...

Flags: needinfo?(nhnt11)
See Also: → 1574196
Flags: needinfo?(mixedpuppy)

I think Gijs is right. Taking this.

Assignee: nobody → nhnt11
Status: NEW → ASSIGNED
Flags: needinfo?(nhnt11)
Priority: -- → P2
Attached image Screenshot with patch

Any reason we can't just add a white-space: pre to the span? This seems to fix the issue for me without messing with existing PanelMultiView consumers. It doesn't affect the text in the menulist item though. (Patch is up as well to demonstrate)

Pushed by nhnt11@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/e0669c56f556
Enable PanelMultiView header text to span multiple lines. r=Gijs,robwu
Status: ASSIGNED → RESOLVED
Closed: 3 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla77
Regressions: 1634813
You need to log in before you can comment on or make changes to this bug.