Closed Bug 1433591 Opened 6 years ago Closed 6 years ago

Links and text selection in Outlook Web Access messages broken, mouse action received by Inbox behind open message

Categories

(Core :: CSS Parsing and Computation, defect)

58 Branch
defect
Not set
normal

Tracking

()

VERIFIED FIXED
mozilla60
Tracking Status
relnote-firefox --- 58+
firefox-esr52 --- unaffected
firefox58 + verified
firefox59 + verified
firefox60 + verified

People

(Reporter: jscher2000, Assigned: emilio)

References

Details

(Keywords: regression)

Attachments

(3 files, 1 obsolete file)

User Agent: Mozilla/5.0 (Windows NT 6.1; Win64; x64; rv:58.0) Gecko/20100101 Firefox/58.0
Build ID: 20180118215408

Steps to reproduce:

On https://outlook.live.com/owa/ with Reading Pane hidden, click message in Inbox, and mouse around over the message. The problem may not appear on the first or second message opened (closing and returning to the inbox in between), but tends to appear eventually.


Actual results:

When the mouse hovers over some links or some parts of links, it shows the default cursor instead of the pointer; when the mouse hovers over some text, it shows the default cursor instead of the insertion point (I-bar) cursor. Clicking when the cursor shows its default appearance removes the message and acts on the Inbox. Similarly attempts to select text using the default cursor show a dragging indicator (moving a message from the Inbox toward something else.


Expected results:

Clicks on the message should be handled in the message, not in the hidden content below it in the z-order.

Note that zooming the message (Ctrl+, Ctrl-) causes Firefox to behave normally on that message.
A related behavior is that the scroll wheel does not scroll the message if the mouse pointer is "over" the text of sender or subject line. 

Same behavior in Nightly 60.0a1 20180126104626
is it possible to reproduce the issue reliably enough to get a regression range with the help of https://mozilla.github.io/mozregression/?
Status: UNCONFIRMED → NEW
Ever confirmed: true
Flags: webcompat?
Keywords: regression
The GUI tool is great, and using a new profile and restoring previous session each time made it less painful. I ran this twice and got the same results. It's hard to know what information to paste here, however. I don't get the definitive statements in the blog post ( https://blog.nightly.mozilla.org/2016/10/11/found-a-regression-in-firefox-give-us-details-with-mozregression/ ).

How about: 

----

app_name: firefox
build_date: 2018-01-05 12:09:38.380000
build_file: C:\...\de5351e9d43f--autoland--target.zip
build_type: inbound
build_url: https://queue.taskcluster.net/v1/task/RGCFUQkqQBu-Tsbm404bgg/runs/0/artifacts/public%2Fbuild%2Ftarget.zip
changeset: de5351e9d43fe8bcbaf7ddeefacba78835405f40
pushlog_url: https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=1eb22728208e6fc9c785e5cf11bb68929d0a555d&tochange=de5351e9d43fe8bcbaf7ddeefacba78835405f40
repo_name: autoland
repo_url: https://hg.mozilla.org/integration/autoland
task_id: RGCFUQkqQBu-Tsbm404bgg

2018-01-27T11:36:54: DEBUG : Using url: https://hg.mozilla.org/integration/autoland/json-pushes?changeset=de5351e9d43fe8bcbaf7ddeefacba78835405f40&full=1
2018-01-27T11:36:54: DEBUG : Found commit message:
Bug 1428164: Reftest. r=me

MozReview-Commit-ID: 5hm56QmtlnV

2018-01-27T11:36:54: INFO : The bisection is done.

----

Hopefully it's useful.
thanks, so this would point towards bug 1428164 as the regressing patch.
Blocks: 1428164
Has Regression Range: --- → yes
Flags: needinfo?(emilio)
Along those lines, setting:

layout.css.stylo-blocklist.enabled => true

layout.css.stylo-blocklist.blocked_domains => live.com

and restarting either Firefox 58.0 or Nightly will work around the issue.

Is it okay for users to manually set this value manually right now? I understand that Mozilla could deploy styloblocklist_signed_v0.2.xpi to modify or overwrite that preference, but since this only appears to affect users who disable the Reading Pane, I'm not sure that would even be used in this case.
I can reproduce the problem on

https://outlook.office365.com/owa/

with the Reading Pane hidden.

Setting

layout.css.stylo-blocklist.enabled => true

layout.css.stylo-blocklist.blocked_domains => live.com,office365.com

works around it.
Tentatively moving to the suspect component. Fascinating, thanks for the report. Good I still have my um.si email account around, otherwise I wouldn't be able to repro it.

I haven't tried yet, will look at it on monday. Thanks!
Component: Untriaged → CSS Parsing and Computation
Product: Firefox → Core
So, I took a closer look (d'oh, really bad at leaving stuff for monday).

The issue here seems to be a DOM like this:

<div style="overflow: hidden">
  <div style="display: table-cell"></div>
</div>

And a parent which gets visibility: hidden.

So it's plausible that bug 1428164 is the regressor... I think before that patch we'd update the anonymous ::-moz-scrolled-content _before_ updating the anonymous wrappers for the table-cell...

I'll try to build a test-case tomorrow (4am here).
Assignee: nobody → emilio
Attached file Testcase
Attachment #8945976 - Attachment is obsolete: true
Flags: needinfo?(emilio)
Attachment #8946099 - Attachment mime type: text/plain → text/html
Arrgh, too bad this was reported on Saturday, if it'd have been on Friday we may have been able to stick the fix on the next 58 dot-release)...

Ritu, Ryan, I know this is a lot to ask, but I'm 110% confident about the fix, and this breaks outlook badly... Is this something we could do?
Flags: needinfo?(ryanvm)
Flags: needinfo?(rkothari)
Comment on attachment 8946098 [details]
Bug 1433591: Update the style of child anon box wrapper after owned anon boxes.

https://reviewboard.mozilla.org/r/216134/#review221878

::: commit-message-c79f9:3
(Diff revision 1)
> +Bug 1433591: Update the style of child anon box wrapper after owned anon boxes. r?bz
> +
> +Since the latter could inherit from the former.

You have "latter" and "former" backwards here, I think.

::: layout/base/ServoRestyleManager.cpp:975
(Diff revision 1)
> -    // Process anon box wrapper frames before ::first-line bits.
> -    childrenRestyleState.ProcessWrapperRestyles(styleFrame);
> -
>      if (wasRestyled) {
>        // Make sure to update anon boxes and pseudo bits after updating text,
>        // otherwise we could clobber first-letter styles from

This is preexisting, but this comment is a bit unclear.  I _think_ it means that ProcessPostTraversalForText could clobber the styles that we pick up from the pseudo update if we did it after the pseudo update, right?  '

But it sounds like it's saying that we could clobber styles that come from ProcessPostTraversalForText... 

Might be worth clarifying the wording here.
Attachment #8946098 - Flags: review?(bzbarsky) → review+
Pushed by ecoal95@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/53d64a077a8e
Update the style of child anon box wrapper after owned anon boxes. r=bz
Comment on attachment 8946098 [details]
Bug 1433591: Update the style of child anon box wrapper after owned anon boxes.

https://reviewboard.mozilla.org/r/216134/#review221878

> You have "latter" and "former" backwards here, I think.

Blerg, one day I'll learn to write English at 5 AM :).

> This is preexisting, but this comment is a bit unclear.  I _think_ it means that ProcessPostTraversalForText could clobber the styles that we pick up from the pseudo update if we did it after the pseudo update, right?  '
> 
> But it sounds like it's saying that we could clobber styles that come from ProcessPostTraversalForText... 
> 
> Might be worth clarifying the wording here.

Yeah, I clarified, thanks a lot for the speedy review!
Comment on attachment 8946100 [details]
Bug 1433591: Test that dynamic changes properly affect hit-testing for elements that have anon boxes.

https://reviewboard.mozilla.org/r/216136/#review221882

::: testing/web-platform/tests/css/cssom-view/elementFromPoint-dynamic-anon-box.html:39
(Diff revision 1)
> +  <div class="inner"></div>
> +</div>
> +<a href="#">Should be clickable</a>
> +<script>
> +test(function() {
> +  document.body.offsetTop;

Might be cleaner to assert that at this point elementFromPoint(10,10) hits one of the divs; that doesn't depend on side-effects offsetTop might have and just directly checks the hit-testing behavior.

::: testing/web-platform/tests/css/cssom-view/elementFromPoint-dynamic-anon-box.html:42
(Diff revision 1)
> +<script>
> +test(function() {
> +  document.body.offsetTop;
> +  document.querySelector('div').style.visibility = "hidden";
> +  assert_equals(document.elementFromPoint(10, 10), document.querySelector('a'));
> +}, "Link should be clickable after hiding an scrollbox with an anonymous table inside");

"a scrollbox"
Attachment #8946100 - Flags: review?(bzbarsky) → review+
Pushed by ecoal95@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/30d915de5462
Test that dynamic changes properly affect hit-testing for elements that have anon boxes. r=bz
(In reply to Emilio Cobos Álvarez [:emilio] from comment #14)
> Ritu, Ryan, I know this is a lot to ask, but I'm 110% confident about the
> fix, and this breaks outlook badly... Is this something we could do?

That's Ritu's call, but my inclination would be to take it for 58.0.2 at this point. Too late IMO to add any extra risk to the 58.0.1 release.
Flags: needinfo?(ryanvm)
https://hg.mozilla.org/mozilla-central/rev/53d64a077a8e
https://hg.mozilla.org/mozilla-central/rev/30d915de5462
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla60
hi chris, apparently this problem could be temporarily worked around by putting outlook on the stylo blocklist (see comment #9). what's the process of getting things rolling there?
Flags: needinfo?(cpeterson)
Comment on attachment 8946098 [details]
Bug 1433591: Update the style of child anon box wrapper after owned anon boxes.

Approval Request Comment
[Feature/Bug causing the regression]: bug 1428164
[User impact if declined]: Outlook being broken.
[Is this code covered by automated tests?]: Yes
[Has the fix been verified in Nightly?]: Yes, just did that.
[Needs manual test from QE? If yes, steps to reproduce]: no
[List of other uplifts needed for the feature/fix]: none
[Is the change risky?]: not risky
[Why is the change risky/not risky?]: it's a very simple change that makes us update the styles in the right order in some edge cases.
[String changes made/needed]: none
Attachment #8946098 - Flags: approval-mozilla-release?
Attachment #8946098 - Flags: approval-mozilla-beta?
in what version will be fixed?
I had an update on firefox but stil not changed with emails on hotmail received for example from linkedin or twoo portals and so on.

If isn't fixed I'll wait but is an important and total blocking problem.

thanks
If you're blocked at the moment, you could make the pref change noted in comment 9 to work around the issue until the real fix ships.
(In reply to alcol from comment #27)
> in what version will be fixed?
> I had an update on firefox but stil not changed with emails on hotmail
> received for example from linkedin or twoo portals and so on.

Emilio is requesting permission (comment 26) to ship a fix in a Firefox 58.0.2 dot release or (more likely) in Firefox 59 (on March 13).
Flags: needinfo?(cpeterson)
Until we have a concrete timeline and plan for 58.0.2, is it worth using domain-controlled pref'ing off of Stylo here? I know we put that mechanism in place for Quantum release.
Flags: needinfo?(rkothari)
Depends on: 1434035
Comment on attachment 8946098 [details]
Bug 1433591: Update the style of child anon box wrapper after owned anon boxes.

Recent regression in Outlook - let's uplift for the 59 beta 6 build.
Attachment #8946098 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
(In reply to jscher2000 from comment #9)
> I can reproduce the problem on
> 
> https://outlook.office365.com/owa/
> 
> with the Reading Pane hidden.
> 
> Setting
> 
> layout.css.stylo-blocklist.enabled => true
> 
> layout.css.stylo-blocklist.blocked_domains => live.com,office365.com
> 
> works around it.

jscher2000, we're looking to use the Stylo domain blocklist to temporarily disable Stylo for Outlook webmail (in bug 1434035). Are live.com and office365.com the only domains we need to disable?
Flags: needinfo?(jscher2000)
(In reply to Chris Peterson [:cpeterson] from comment #33)
> jscher2000, we're looking to use the Stylo domain blocklist to temporarily
> disable Stylo for Outlook webmail (in bug 1434035). Are live.com and
> office365.com the only domains we need to disable?

Let's add office.com (for https://outlook.office.com/owa/) for a total of three.
Unfortunately, it looks like Outlook Web Access (OWA) can be hosted on company subdomains, so we won't be able to disable Stylo for all OWA domains.

Some examples:

https://owa.house.gov/owa/
http://www.halliburton.com/owa.html
https://owa.marriott.com/jump/
Flags: needinfo?(jscher2000)
Summary: Links and text selection in Outlook messages broken, mouse action received by Inbox behind open message → Links and text selection in Outlook Web Access messages broken, mouse action received by Inbox behind open message
Flags: in-testsuite+
hi  Emilio Cobos Álvarez [:emilio], it says fixed in Firefox 60, when I go to update Firefox I'm taken to a page that says Firefox 58 is the latest version and I'm up to date... This is not fixed for me MusicMan0070  Bug 1433826
(In reply to Eddie deSouza from comment #38)
> hi  Emilio Cobos Álvarez [:emilio], it says fixed in Firefox 60, when I go
> to update Firefox I'm taken to a page that says Firefox 58 is the latest
> version and I'm up to date... This is not fixed for me MusicMan0070  Bug
> 1433826

Yeah, that's known. Firefox 60 is effectively Nightly right now (http://nightly.mozilla.org/). This should also be fixed on Beta and Dev Edition, but not on 58, which is "release".

I'm awaiting a decision about what to do with that. If you want to try nightly or beta, you're very welcome to :)
Note that the first Beta to include this fix is 59b6 coming later this week.
(In reply to Eddie deSouza from comment #38)
> hi  Emilio Cobos Álvarez [:emilio], it says fixed in Firefox 60, when I go
> to update Firefox I'm taken to a page that says Firefox 58 is the latest
> version and I'm up to date... This is not fixed for me MusicMan0070

Hi Eddie, I don't recommend switching versions over this issue. The following Mozilla Support thread has a good summary of the currently known workarounds for the problem: 

https://support.mozilla.org/questions/1202259
Our current plan is to ship this Outlook fix in a Firefox 58.0.2 dot release (hopefully next week) instead of rushing the system add-on out this week.

Because of the way the system add-on would have worked, it would only fix the bug for users on Microsoft's outlook.com/etc domains, not any of the enterprise users with their on-premise Outlook websites. The system add-on fix also introduced some risk because we would be re-activating Firefox's pre-Quantum CSS code, which might reveal new surprising bugs with Outlook. Better to ship a safer fix for everyone next week.

My current understanding of the bug is that it just prevents the user from clicking on email links or scrolling with mouse wheel or trackpad when the mouse pointer is in a roughly 1"x1" region in the upper left corner of the email. If the link extends wider than 1" or is located lower on the screen, then it is clickable. If the user tries to click within the 1"x1" region, the email view is closed and Outlook returns to the inbox view.

If the user enables the Outlook reading pane or the new Outlook beta UX, the bug goes away. I could reproduce the bug on my Windows 10 laptop, but not on an external monitor or on my MacBook Pro.
(In reply to Chris Peterson [:cpeterson] from comment #42)
> My current understanding of the bug is that it just prevents the user from
> clicking on email links or scrolling with mouse wheel or trackpad when the
> mouse pointer is in a roughly 1"x1" region in the upper left corner of the
> email. If the link extends wider than 1" or is located lower on the screen,
> then it is clickable. If the user tries to click within the 1"x1" region,
> the email view is closed and Outlook returns to the inbox view.

The region of mouse dysfunction can be very large, but varies based on your message list contents. Wherever there is text in the sender name and subject columns, the mouse pointer will be acting on that text instead of on the message the user sees. Clicks activate the message in the message list, and the scroll wheel/gesture scrolls the message list. I see the same on OSX 10.12 as on Windows 7.

Reading Pane, OWA Beta, and Stylo blocklist are the three workarounds we'll continue to mention on SuMo.
(In reply to jscher2000 from comment #43)
> Reading Pane, OWA Beta, and Stylo blocklist are the three workarounds we'll
> continue to mention on SuMo.

Cool. Thank you for all your help isolating this bug and supporting the users reporting problems! :)
I found another fix, just right click on the link and open in new tab or window, it's worked 3 time for me now so thought I would share it with you all... Eddie
Comment on attachment 8946098 [details]
Bug 1433591: Update the style of child anon box wrapper after owned anon boxes.

This bug is driving 58.0.2. Release58+
Attachment #8946098 - Flags: approval-mozilla-release? → approval-mozilla-release+
Flags: qe-verify+
I have managed to reproduce the issue mentioned in comment 0 (with the attachment from comment 13) using Firefox 60.0a1 (BuildId:20180126220105).

This issue is verified fixed using Firefox 60.0a1 (BuildId:20180206220102), 59.0b7 (BuildId:20180205211730) and 58.0.2 (BuildId:20180206200532) on Windows 10 64bit, macOS 10.12.6 and Ubuntu 16.04 64bit.
Status: RESOLVED → VERIFIED
Flags: qe-verify+
Release Note Request (optional, but appreciated)
[Why is this notable]: This fixes Outlook and Hotmail.
[Affects Firefox for Android]: No because Stylo is not enabled in Firefox 58 for Android.
[Suggested wording]:

Fix clicking links and scrolling emails on Microsoft Hotmail and Outlook (OWA) webmail. (bug 1433591)

[Links (documentation, blog post, etc)]: No docs. Just this bug.
relnote-firefox: --- → ?
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: