1596356 - Remove XUL absolute positioning attribute code

legacy-stack, the only consumer of display: -moz-stack, doesn't use any of those attributes, let's remove them. Every other stack conversion has baked for about a year now.

Pulsebot

Comment 3

•

5 years ago

Pushed by ntim.bugs@gmail.com: https://hg.mozilla.org/integration/autoland/rev/88c7b1169eab Remove XUL absolute positioning attribute code. r=emilio

Narcis Beleuzu [:NarcisB]

Comment 4

•

5 years ago

Backed out for mochitest failure on test_largemenu.html

Backout link: https://hg.mozilla.org/integration/autoland/rev/e90ad3f0ee5d286fbba0575481c580a57995b389
Log link: https://treeherder.mozilla.org/logviewer?job_id=320484347&repo=autoland&lineNumber=4656

Flags: needinfo?(ntim.bugs)

Tim Nguyen :ntim

Assignee

Updated

•

5 years ago

Flags: needinfo?(ntim.bugs)

Pulsebot

Comment 5

•

5 years ago

Pushed by ntim.bugs@gmail.com: https://hg.mozilla.org/integration/autoland/rev/e942a748de2d Remove XUL absolute positioning attribute code. r=emilio

Bogdan Tara[:bogdan_tara | bogdant]

Comment 6

•

5 years ago

Backed out changeset e942a748de2d (bug 1596356) for test_largemenu.html failures.

Push with failures: https://treeherder.mozilla.org/jobs?repo=autoland&group_state=expanded&selectedTaskRun=d5jE-PqURiiYswshtC3mKg.0&fromchange=57fcc7dd2219f3e7a194c896cb7e3c186adb0c44&searchStr=mochitest-chrome-1proc&tochange=7dab99ba8ca3cd7a5a6a15b86c22dc39cbb5a4d0

Backout link: https://hg.mozilla.org/integration/autoland/rev/7dab99ba8ca3cd7a5a6a15b86c22dc39cbb5a4d0

Failure log: https://treeherder.mozilla.org/logviewer?job_id=320519179&repo=autoland&lineNumber=1820

[task 2020-11-02T22:16:38.243Z] 22:16:38     INFO - TEST-START | toolkit/content/tests/chrome/test_largemenu.html
[task 2020-11-02T22:16:38.625Z] 22:16:38     INFO - TEST-INFO | started process screencapture
[task 2020-11-02T22:16:38.741Z] 22:16:38     INFO - TEST-INFO | screencapture: exit 0
[task 2020-11-02T22:16:38.741Z] 22:16:38     INFO - Buffered messages logged at 22:16:38
[task 2020-11-02T22:16:38.741Z] 22:16:38     INFO - must wait for load
[task 2020-11-02T22:16:38.741Z] 22:16:38     INFO - must wait for focus
[task 2020-11-02T22:16:38.742Z] 22:16:38     INFO - TEST-PASS | toolkit/content/tests/chrome/test_largemenu.html | open normal top 
[task 2020-11-02T22:16:38.742Z] 22:16:38     INFO - TEST-PASS | toolkit/content/tests/chrome/test_largemenu.html | open normal bottom 
[task 2020-11-02T22:16:38.742Z] 22:16:38     INFO - TEST-PASS | toolkit/content/tests/chrome/test_largemenu.html | open normal overflow 
[task 2020-11-02T22:16:38.742Z] 22:16:38     INFO - TEST-PASS | toolkit/content/tests/chrome/test_largemenu.html | menu scroll position 
[task 2020-11-02T22:16:38.742Z] 22:16:38     INFO - TEST-PASS | toolkit/content/tests/chrome/test_largemenu.html | open when bottom would overlap top 
[task 2020-11-02T22:16:38.742Z] 22:16:38     INFO - TEST-PASS | toolkit/content/tests/chrome/test_largemenu.html | open when bottom would overlap bottom 
[task 2020-11-02T22:16:38.742Z] 22:16:38     INFO - TEST-PASS | toolkit/content/tests/chrome/test_largemenu.html | open when bottom would overlap overflow 
[task 2020-11-02T22:16:38.742Z] 22:16:38     INFO - TEST-PASS | toolkit/content/tests/chrome/test_largemenu.html | menu scroll position 
[task 2020-11-02T22:16:38.743Z] 22:16:38     INFO - TEST-PASS | toolkit/content/tests/chrome/test_largemenu.html | open with scrolling top 
[task 2020-11-02T22:16:38.743Z] 22:16:38     INFO - TEST-PASS | toolkit/content/tests/chrome/test_largemenu.html | open with scrolling bottom 
[task 2020-11-02T22:16:38.743Z] 22:16:38     INFO - TEST-PASS | toolkit/content/tests/chrome/test_largemenu.html | open with scrolling overflow 
[task 2020-11-02T22:16:38.743Z] 22:16:38     INFO - TEST-PASS | toolkit/content/tests/chrome/test_largemenu.html | menu scroll position 
[task 2020-11-02T22:16:38.743Z] 22:16:38     INFO - TEST-PASS | toolkit/content/tests/chrome/test_largemenu.html | menu scroll position 
[task 2020-11-02T22:16:38.743Z] 22:16:38     INFO - TEST-PASS | toolkit/content/tests/chrome/test_largemenu.html | open small again top 
[task 2020-11-02T22:16:38.743Z] 22:16:38     INFO - TEST-PASS | toolkit/content/tests/chrome/test_largemenu.html | open small again bottom 
[task 2020-11-02T22:16:38.744Z] 22:16:38     INFO - TEST-PASS | toolkit/content/tests/chrome/test_largemenu.html | open small again overflow 
[task 2020-11-02T22:16:38.744Z] 22:16:38     INFO - TEST-PASS | toolkit/content/tests/chrome/test_largemenu.html | menu scroll position 
[task 2020-11-02T22:16:38.744Z] 22:16:38     INFO - TEST-PASS | toolkit/content/tests/chrome/test_largemenu.html | menu movement (1, 1) x 
[task 2020-11-02T22:16:38.744Z] 22:16:38     INFO - TEST-PASS | toolkit/content/tests/chrome/test_largemenu.html | menu movement (1, 1) y 
[task 2020-11-02T22:16:38.744Z] 22:16:38     INFO - TEST-PASS | toolkit/content/tests/chrome/test_largemenu.html | menu movement (100, 8000) x 
[task 2020-11-02T22:16:38.744Z] 22:16:38     INFO - TEST-PASS | toolkit/content/tests/chrome/test_largemenu.html | menu movement (100, 8000) y 
[task 2020-11-02T22:16:38.744Z] 22:16:38     INFO - TEST-PASS | toolkit/content/tests/chrome/test_largemenu.html | menu movement (6000, 100) x 
[task 2020-11-02T22:16:38.744Z] 22:16:38     INFO - TEST-PASS | toolkit/content/tests/chrome/test_largemenu.html | menu movement (6000, 100) y 
[task 2020-11-02T22:16:38.744Z] 22:16:38     INFO - TEST-PASS | toolkit/content/tests/chrome/test_largemenu.html | menu movement left is empty after moving 
[task 2020-11-02T22:16:38.745Z] 22:16:38     INFO - TEST-PASS | toolkit/content/tests/chrome/test_largemenu.html | menu movement top is empty after moving 
[task 2020-11-02T22:16:38.745Z] 22:16:38     INFO - TEST-PASS | toolkit/content/tests/chrome/test_largemenu.html | menu movement set left and top x 
[task 2020-11-02T22:16:38.745Z] 22:16:38     INFO - TEST-PASS | toolkit/content/tests/chrome/test_largemenu.html | menu movement set left and top y 
[task 2020-11-02T22:16:38.745Z] 22:16:38     INFO - TEST-PASS | toolkit/content/tests/chrome/test_largemenu.html | menu movement move after set left and top x 
[task 2020-11-02T22:16:38.745Z] 22:16:38     INFO - TEST-PASS | toolkit/content/tests/chrome/test_largemenu.html | menu movement move after set left and top y 
[task 2020-11-02T22:16:38.745Z] 22:16:38     INFO - TEST-PASS | toolkit/content/tests/chrome/test_largemenu.html | menu movement left is set after moving 
[task 2020-11-02T22:16:38.745Z] 22:16:38     INFO - TEST-PASS | toolkit/content/tests/chrome/test_largemenu.html | menu movement top is set after moving 
[task 2020-11-02T22:16:38.745Z] 22:16:38     INFO - TEST-PASS | toolkit/content/tests/chrome/test_largemenu.html | menu movement move after set left and top x to -1 
[task 2020-11-02T22:16:38.745Z] 22:16:38     INFO - TEST-PASS | toolkit/content/tests/chrome/test_largemenu.html | menu movement move after set left and top y to -1 
[task 2020-11-02T22:16:38.745Z] 22:16:38     INFO - TEST-PASS | toolkit/content/tests/chrome/test_largemenu.html | menu movement left is not set after moving to -1 
[task 2020-11-02T22:16:38.745Z] 22:16:38     INFO - TEST-PASS | toolkit/content/tests/chrome/test_largemenu.html | menu movement top is not set after moving to -1 
[task 2020-11-02T22:16:38.745Z] 22:16:38     INFO - TEST-PASS | toolkit/content/tests/chrome/test_largemenu.html | panel movement (1, 1) x 
[task 2020-11-02T22:16:38.746Z] 22:16:38     INFO - TEST-PASS | toolkit/content/tests/chrome/test_largemenu.html | panel movement (1, 1) y 
[task 2020-11-02T22:16:38.746Z] 22:16:38     INFO - TEST-PASS | toolkit/content/tests/chrome/test_largemenu.html | panel movement (100, 8000) x 
[task 2020-11-02T22:16:38.746Z] 22:16:38     INFO - TEST-PASS | toolkit/content/tests/chrome/test_largemenu.html | panel movement (100, 8000) y 
[task 2020-11-02T22:16:38.746Z] 22:16:38     INFO - TEST-PASS | toolkit/content/tests/chrome/test_largemenu.html | panel movement (6000, 100) x 
[task 2020-11-02T22:16:38.746Z] 22:16:38     INFO - TEST-PASS | toolkit/content/tests/chrome/test_largemenu.html | panel movement (6000, 100) y 
[task 2020-11-02T22:16:38.746Z] 22:16:38     INFO - TEST-PASS | toolkit/content/tests/chrome/test_largemenu.html | panel movement left is empty after moving 
[task 2020-11-02T22:16:38.746Z] 22:16:38     INFO - TEST-PASS | toolkit/content/tests/chrome/test_largemenu.html | panel movement top is empty after moving 
[task 2020-11-02T22:16:38.746Z] 22:16:38     INFO - TEST-PASS | toolkit/content/tests/chrome/test_largemenu.html | panel movement set left and top x 
[task 2020-11-02T22:16:38.746Z] 22:16:38     INFO - TEST-PASS | toolkit/content/tests/chrome/test_largemenu.html | panel movement set left and top y 
[task 2020-11-02T22:16:38.746Z] 22:16:38     INFO - TEST-PASS | toolkit/content/tests/chrome/test_largemenu.html | panel movement move after set left and top x 
[task 2020-11-02T22:16:38.746Z] 22:16:38     INFO - TEST-PASS | toolkit/content/tests/chrome/test_largemenu.html | panel movement move after set left and top y 
[task 2020-11-02T22:16:38.746Z] 22:16:38     INFO - TEST-PASS | toolkit/content/tests/chrome/test_largemenu.html | panel movement left is set after moving 
[task 2020-11-02T22:16:38.746Z] 22:16:38     INFO - TEST-PASS | toolkit/content/tests/chrome/test_largemenu.html | panel movement top is set after moving 
[task 2020-11-02T22:16:38.747Z] 22:16:38     INFO - Buffered messages finished
[task 2020-11-02T22:16:38.747Z] 22:16:38     INFO - TEST-UNEXPECTED-FAIL | toolkit/content/tests/chrome/test_largemenu.html | panel movement move after set left and top x to -1 - got -1, expected +0
[task 2020-11-02T22:16:38.747Z] 22:16:38     INFO - SimpleTest.is@chrome://mochikit/content/tests/SimpleTest/SimpleTest.js:500:14
[task 2020-11-02T22:16:38.747Z] 22:16:38     INFO - is@chrome://mochitests/content/chrome/toolkit/content/tests/chrome/window_largemenu.xhtml:163:55
[task 2020-11-02T22:16:38.747Z] 22:16:38     INFO - testPopupMovement@chrome://mochitests/content/chrome/toolkit/content/tests/chrome/window_largemenu.xhtml:381:5
[task 2020-11-02T22:16:38.747Z] 22:16:38     INFO - onpopupshown@chrome://mochitests/content/chrome/toolkit/content/tests/chrome/window_largemenu.xhtml:1:1
[task 2020-11-02T22:16:38.747Z] 22:16:38     INFO - TEST-PASS | toolkit/content/tests/chrome/test_largemenu.html | panel movement move after set left and top y to -1 
[task 2020-11-02T22:16:38.747Z] 22:16:38     INFO - TEST-PASS | toolkit/content/tests/chrome/test_largemenu.html | panel movement left is not set after moving to -1 
[task 2020-11-02T22:16:38.747Z] 22:16:38     INFO - TEST-PASS | toolkit/content/tests/chrome/test_largemenu.html | panel movement top is not set after moving to -1 
[task 2020-11-02T22:16:38.801Z] 22:16:38     INFO - TEST-PASS | toolkit/content/tests/chrome/test_largemenu.html | context menu enough space below top 
[task 2020-11-02T22:16:38.802Z] 22:16:38     INFO - TEST-PASS | toolkit/content/tests/chrome/test_largemenu.html | context menu enough space below left 
[task 2020-11-02T22:16:39.016Z] 22:16:39     INFO - TEST-PASS | toolkit/content/tests/chrome/test_largemenu.html | context menu more space above top 
[task 2020-11-02T22:16:39.016Z] 22:16:39     INFO - TEST-PASS | toolkit/content/tests/chrome/test_largemenu.html | context menu more space above left 
[task 2020-11-02T22:16:39.244Z] 22:16:39     INFO - TEST-PASS | toolkit/content/tests/chrome/test_largemenu.html | context menu too big either side top 
[task 2020-11-02T22:16:39.244Z] 22:16:39     INFO - TEST-PASS | toolkit/content/tests/chrome/test_largemenu.html | context menu too big either side left 
[task 2020-11-02T22:16:39.266Z] 22:16:39     INFO - TEST-PASS | toolkit/content/tests/chrome/test_largemenu.html | context menu larger than screen top 
[task 2020-11-02T22:16:39.266Z] 22:16:39     INFO - TEST-PASS | toolkit/content/tests/chrome/test_largemenu.html | context menu larger than screen left 
[task 2020-11-02T22:16:39.476Z] 22:16:39     INFO - TEST-PASS | toolkit/content/tests/chrome/test_largemenu.html | context menu flips horizontally on osx right 
[task 2020-11-02T22:16:39.516Z] 22:16:39     INFO - GECKO(1906) | MEMORY STAT | vsize 7692MB | residentFast 357MB | heapAllocated 154MB
[task 2020-11-02T22:16:39.520Z] 22:16:39     INFO - TEST-OK | toolkit/content/tests/chrome/test_largemenu.html | took 1287ms

Flags: needinfo?(ntim.bugs)

Tim Nguyen :ntim

Assignee

Comment 7

•

5 years ago

•

Edited

Hi Emilio, there's only one failure left in test_largemenu.html:

[task 2020-11-02T22:16:38.747Z] 22:16:38     INFO - Buffered messages finished
[task 2020-11-02T22:16:38.747Z] 22:16:38     INFO - TEST-UNEXPECTED-FAIL | toolkit/content/tests/chrome/test_largemenu.html | panel movement move after set left and top x to -1 - got -1, expected +0
[task 2020-11-02T22:16:38.747Z] 22:16:38     INFO - SimpleTest.is@chrome://mochikit/content/tests/SimpleTest/SimpleTest.js:500:14
[task 2020-11-02T22:16:38.747Z] 22:16:38     INFO - is@chrome://mochitests/content/chrome/toolkit/content/tests/chrome/window_largemenu.xhtml:163:55
[task 2020-11-02T22:16:38.747Z] 22:16:38     INFO - testPopupMovement@chrome://mochitests/content/chrome/toolkit/content/tests/chrome/window_largemenu.xhtml:381:5
[task 2020-11-02T22:16:38.747Z] 22:16:38     INFO - onpopupshown@chrome://mochitests/content/chrome/toolkit/content/tests/chrome/window_largemenu.xhtml:1:1

Do you think I forgot to update an usage of .left/.top setter/getters or that left/top attributes still need reflows for nsMenuPopupFrame?

Flags: needinfo?(ntim.bugs) → needinfo?(emilio)

Emilio Cobos Álvarez [:emilio]

Comment 8

•

5 years ago

Very easy to check. This does indeed fix the issue:

diff --git a/layout/xul/nsMenuPopupFrame.cpp b/layout/xul/nsMenuPopupFrame.cpp
index 3768ab88bd9b..f8ec9867c07a 100644
--- a/layout/xul/nsMenuPopupFrame.cpp
+++ b/layout/xul/nsMenuPopupFrame.cpp
@@ -2239,8 +2239,9 @@ nsresult nsMenuPopupFrame::AttributeChanged(int32_t aNameSpaceID,
   nsresult rv =
       nsBoxFrame::AttributeChanged(aNameSpaceID, aAttribute, aModType);
 
-  if (aAttribute == nsGkAtoms::left || aAttribute == nsGkAtoms::top)
+  if (aAttribute == nsGkAtoms::left || aAttribute == nsGkAtoms::top) {
     MoveToAttributePosition();
+  }
 
   if (aAttribute == nsGkAtoms::remote) {
     // When the remote attribute changes, we need to create a new widget to
@@ -2295,6 +2296,9 @@ void nsMenuPopupFrame::MoveToAttributePosition() {
   mozilla::CSSIntPoint pos(left.ToInteger(&err1), top.ToInteger(&err2));
 
   if (NS_SUCCEEDED(err1) && NS_SUCCEEDED(err2)) MoveTo(pos, false);
+
+  PresShell()->FrameNeedsReflow(this, IntrinsicDirty::StyleChange,
+                                NS_FRAME_IS_DIRTY);
 }
 
 void nsMenuPopupFrame::DestroyFrom(nsIFrame* aDestructRoot,

rs=me with that.

Flags: needinfo?(emilio)

Tim Nguyen :ntim

Assignee

Comment 9

•

5 years ago

Thanks for your help!

Pulsebot

Comment 10

•

5 years ago

Pushed by ntim.bugs@gmail.com: https://hg.mozilla.org/integration/autoland/rev/c46ecd514ba1 Remove XUL absolute positioning attribute code. r=emilio

Cristian Brindusan [:cbrindusan]

Comment 11

•

5 years ago

bugherder

https://hg.mozilla.org/mozilla-central/rev/c46ecd514ba1

Status: ASSIGNED → RESOLVED

Closed: 5 years ago

status-firefox84: --- → fixed

Resolution: --- → FIXED

Target Milestone: --- → 84 Branch

Tim Nguyen :ntim

Assignee

Comment 12

•

5 years ago

Hi, just a heads up that we're removing .left/.top getter/setter support from elements with display: -moz-popup. These can be replaced with .getAttribute("left")/".getAttribute("top")or thesetAttribute` equivalent.

Flags: needinfo?(richard.marti)

Flags: needinfo?(mkmelin+mozilla)

Richard Marti (:Paenglab)

Comment 13

•

5 years ago

Thanks Tim, for the heads up. Magnus, can you check this?

Flags: needinfo?(richard.marti)

Magnus Melin [:mkmelin]

Comment 14

•

5 years ago

It looks like we don't have any usage of this left.

Flags: needinfo?(mkmelin+mozilla)

Bugzilla

Remove XUL absolute positioning attribute code

Categories

(Core :: Layout, task)

Tracking

()

People

(Reporter: ntim, Assigned: ntim)

References

(Blocks 1 open bug)

Details

Crash Data

Security

(public)

User Story

Attachments

(1 file)

Description

Updated

Comment 1

Updated

Comment 2

Comment 3

Comment 4

Updated

Comment 5

Comment 6

Comment 7

Comment 8

Comment 9

Comment 10

Comment 11

Comment 12

Comment 13

Comment 14

Attachment

General

Description

File Name

Content Type