Closed Bug 1348678 Opened 4 years ago Closed 4 years ago

No display of link target when the statusbar is hidden.

Categories

(Thunderbird :: Message Reader UI, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
Thunderbird 55.0

People

(Reporter: Paenglab, Assigned: Paenglab)

Details

Attachments

(1 file, 4 obsolete files)

When the statusbar is hidden, the user has no possibility to preview the true link target of a link in a message. This could be a security risk for the user.
Attached patch showLink.patch (obsolete) — Splinter Review
This is a port of FX bug 628654.

The status panel will be only used, when the statusbar is hidden. The links are still displayed in the statusbar when the bar is shown.

We could also add all other messages from the statusbar, but I think when the user has hidden the bar he doesn't want to see them.
Assignee: nobody → richard.marti
Status: NEW → ASSIGNED
Attachment #8848894 - Flags: review?(mkmelin+mozilla)
Attached patch showLink.patch (obsolete) — Splinter Review
Added comments in mailWindow.js
Attachment #8848894 - Attachment is obsolete: true
Attachment #8848894 - Flags: review?(mkmelin+mozilla)
Attachment #8848895 - Flags: review?(mkmelin+mozilla)
Comment on attachment 8848895 [details] [diff] [review]
showLink.patch

Review of attachment 8848895 [details] [diff] [review]:
-----------------------------------------------------------------

Some drive-by comments, all style nits. They hate me for picking on comments ;-)

::: mail/base/content/mailWindow.js
@@ +246,5 @@
>    },
>  
>    setOverLink: function(link, context) {
> +    // Is the statusbar visible?
> +    if (!document.getElementById("status-bar").hidden) {

No point commenting to obvious.

@@ +247,5 @@
>  
>    setOverLink: function(link, context) {
> +    // Is the statusbar visible?
> +    if (!document.getElementById("status-bar").hidden) {
> +      // Yes, show the link in it

I'd put:
// Statusbar visible: Show link in it.

@@ +250,5 @@
> +    if (!document.getElementById("status-bar").hidden) {
> +      // Yes, show the link in it
> +      this._statusText.label = link;
> +    } else {
> +      // No, show the link in the statuspanel

// Statusbar invisible: Show link in statuspanel instead.
Attached patch showLink.patch (obsolete) — Splinter Review
Fixed the comments.
Attachment #8848895 - Attachment is obsolete: true
Attachment #8848895 - Flags: review?(mkmelin+mozilla)
Attachment #8849066 - Flags: review?(mkmelin+mozilla)
Comment on attachment 8849066 [details] [diff] [review]
showLink.patch

I'll take the review since I've already commented.
Attachment #8849066 - Flags: review?(mkmelin+mozilla) → review?(jorgk)
Attached image Screenshot (obsolete) —
Looks like there is something wrong with the transparency. The vertical border of the folder pane is still visible. The scrollbar is covered.
Attached patch showLink.patchSplinter Review
It wasn't a transparency issue. The splitter was layered before the statuspanel.
Attachment #8852119 - Attachment is obsolete: true
Attachment #8852134 - Flags: review?(jorgk)
Attachment #8849066 - Attachment is obsolete: true
Attachment #8849066 - Flags: review?(jorgk)
Comment on attachment 8852134 [details] [diff] [review]
showLink.patch

Works for me, code looks good. r=jorgk.
Attachment #8852134 - Flags: review?(jorgk) → review+
Keywords: checkin-needed
https://hg.mozilla.org/comm-central/rev/e598d29b42ce38f4a37910ae960a2d6597320cbc
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 55.0
You need to log in before you can comment on or make changes to this bug.