Currently, invalid form popup points to the left of the element (maybe the right with RTL, I don't know). We should probably make it point to the middle to prevent potential issues with RTL and to prevent positions issues when the elements are completely restyled and have a padding like in this screenshot: http://fredericiana.com/wp-content/uploads/2010/12/html5-feedback-forms-1.jpg

Thanks for filing this followup bug for me, and apologies that I thought it was the same problem as bug 616607!

Actually, I don't think we should point to the middle but we can just take into account borders and padding. It's quite simple (I already have a patch doing that). In addition, we should make sure to point to right of the element if it's in RTL mode (it's currently not working nicely in RTL). However, Neil, for unknown reasons, if I do openPopup(element, "after_start", w, h) with w and h being negative values, the arrow of the arrow panel disappeared. Is that a simple bug or there are some reasons by design?

Ehsan, do we agree that it would be better to have invalid form popup showing at the right of the element (near the cursor) when in RTL mode? Though, the arrow will still be at the left of the popup. Would that be all right? Given that I do not use RTL, I prefer not to do wrong assumptions here.

With this patch, the popup has now an offset depending on the border and the padding so the arrow will point just below the content. For checkbox and radio buttons, the arrow will point at the center. When the element is in RTL, the arrow should be on the right of the popup and point on the right of the element (or the center). However, I think there is a bug: sometimes the panel is positioned as asked but the arrow hasn't the correct position. Enn, let me know if you have any idea why I have this bug. This only happen in RTL when I ask to have the arrow on the right (the arrow is on the left but the panel is positioned to have it on the right). Simon, could you tell me if what is done for RTL is what you would expect?

The page in the URL field can be used to test this patch. Note that the bug mentioned in comment 4 can be seen by switching to RTL with the checkbox then submitting the form: the bug will happen for all text fields but not for checkbox and radio buttons.

Are you testing this on a page that has rtl direction or when the browser UI has rtl direction? Or, better, both?

Gasp, the browser UI direction changes how the arrow panel behave :-/ So, with the current patch and: 1. brower UI is RTL but not the content: the panel is positioned at the right and the arrow is on the left but should be on the right ; 2. browser UI is RTL and the content is RTL: the panel is on the left and the arrow is on the left ; 3. browser UI is not RTL but the content is: the panel is positioned at the right and the arrow is on the left but should be on the right ; 4. browser UI is not RTL and content is not RTL: the panel is on the left and the arrow is on the left. So what happens here is that my patch is trying to detect if we are in RTL mode and change how the panel should appear but I guess the panel code does the same so in some situations it doesn't behave as expected. I would say that the arrow should take the direction from the content and not the browser UI given that having an RTL popup in a LTR page might be weird. In addition, the content can redefine the message inside the popup which can lead to other issues :( For the bug, Neil, I think it is persistent. Obviously cases 1 and 4 should have the panel at the right and the arrow at the right. Maybe we should block on this for the RTL issue?

It may just be the case that the three places in nsMenuPopupFrame.cpp and the one place in popup.xml where the direction is used just need to be changed to use the anchor's direction rather than the popup's direction. Thus, the rtl parts of your patch may not be needed.

This is what happens with the changes recommended in comment 8 (copied from IRC): [16:15] < volkmar> Enn: with the browser UI in LTR, everything is fixe except a bug when the arrow should be in "bottomcenter topleft" in RTL (which is topright then) : the panel is positionned to be in topright but the arrow is in topleft [16:15] < volkmar> as a consequence, the arrow points nowhere [16:16] < volkmar> Enn: let me know if you need some screenshots, i doubt that my description are clear [16:18] < volkmar> Enn: and when the browser UI is in RTL, the panels are always at the correct position but the arrow are most of the time misplaced In addition, when the browser UI is in RTL, the notification popup direction changed but I guess this could be fixed by inverting the direction before calling openPopup?

Comment on attachment 500323 [details] [diff] [review] Patch v1 What should happen IMO is this: The directionality of the text in the pop-up should be inherited from the direction of the browser (so with RTL browser UI, it should appear as ".Please fill out this field" with the period on the left). This is working correctly. The position of the pop-up and the arrow should depend only on the directionality of the field that it's attached to. The arrow should be at the "start" of the field, and the pop-up should be aligned to the "start", where "start"=left for a LTR field and right for a RTL field. FWIW, this works correctly in trunk for the "remember password" pop-up (bug 613843 and bug 606343).

FWIW, I agree with Simon in comment 10.

I split the bug. This will remain for the RTL issues. The original issue is now in bug 624151. Asking a blocker status given that RTL users might have a bad experience with the current implementation of the invalid form popup.

This patch should do what Simon and Ehsan are requesting: The content direction of the panels is following the general UI direction but the arrow position depends on the content direction. It also prevents the workaround for the identity, password and geolocation panels direction.

Better with a real patch...

Neil, you should note that there is still one bug with the arrow panels which is unrelated to the patch. If you try the the second form (in RTL) in the URL field with this patch applied, the arrow panel will not apply correctly: at the right but the arrow at the left.

Comment on attachment 502269 [details] [diff] [review] Patch v2 This looks good, modulo the issue mentioned in comment 15

(In reply to comment #15) > Neil, you should note that there is still one bug with the arrow panels which > is unrelated to the patch. If you try the the second form (in RTL) in the URL > field with this patch applied, the arrow panel will not apply correctly: at the > right but the arrow at the left. Which arrow panel are you referring to and what is the 'second form'?

The values for position should be changed to "end" and "start" values (e.g. bottomstart, topend, etc.). Either way, this needs documentation and is somewhat of a compat change.

This patch actually breaks layout/xul/base/test/test_bug477754.xul See: http://tinderbox.mozilla.org/showlog.cgi?log=MozillaTry/1294748904.1294753151.31939.gz#err0

(In reply to comment #17) > (In reply to comment #15) > > Neil, you should note that there is still one bug with the arrow panels which > > is unrelated to the patch. If you try the the second form (in RTL) in the URL > > field with this patch applied, the arrow panel will not apply correctly: at the > > right but the arrow at the left. > > Which arrow panel are you referring to and what is the 'second form'? The RTL one. By the way, it seems like test_bug477754.xul tests something that doesn't really apply anymore (the customize panel is centered). Do we still want this test? or I'm missing the point of it?

(In reply to comment #20) > > By the way, it seems like test_bug477754.xul tests something that doesn't > really apply anymore (the customize panel is centered). Do we still want this > test? or I'm missing the point of it? That test isn't testing the customize panel, it tests the underlying cause of the bug.

(In reply to comment #21) > (In reply to comment #20) > > > > By the way, it seems like test_bug477754.xul tests something that doesn't > > really apply anymore (the customize panel is centered). Do we still want this > > test? or I'm missing the point of it? > > That test isn't testing the customize panel, it tests the underlying cause of > the bug. So, making the anchor RTL instead of the popup fixes this test.

Pushed: http://hg.mozilla.org/mozilla-central/rev/7fcae0c7f36a

I don't see any obvious new API here, other than what looks like an internal IsDirectionRTL() function. Is the documentation point here to simply say the popup hangs the opposite direction for RTL locales?

(In reply to comment #15) > Neil, you should note that there is still one bug with the arrow panels which > is unrelated to the patch. If you try the the second form (in RTL) in the URL > field with this patch applied, the arrow panel will not apply correctly: at the > right but the arrow at the left. If that bug is unrelated to this patch, does it have its own bug?

(In reply to comment #25) > (In reply to comment #15) > > Neil, you should note that there is still one bug with the arrow panels which > > is unrelated to the patch. If you try the the second form (in RTL) in the URL > > field with this patch applied, the arrow panel will not apply correctly: at the > > right but the arrow at the left. > > If that bug is unrelated to this patch, does it have its own bug? Sorry, I should have open a bug. It's now done: bug 626605. But since this patch is pushed I'm trying to understand why the behavior is weird but I don't understand the arrow panel code enough yet.

Added a note here: https://developer.mozilla.org/en/HTML/HTML5/Forms_in_HTML5#Constraint_Validation

Eric, I don't think the dev-doc-needed flag was for HTML5 Forms but for the XUL panel which now depends on the direction of the anchor.

Oh. That wasn't clear at all from any of the discussion or the patch. Thanks for catching my error! Updated: https://developer.mozilla.org/en/XUL/Attribute/panel.type Hopefully that covers it correctly; otherwise I'm stumped. :)

(In reply to comment #29) > Oh. That wasn't clear at all from any of the discussion or the patch. Thanks > for catching my error! > > Updated: > > https://developer.mozilla.org/en/XUL/Attribute/panel.type > > Hopefully that covers it correctly; otherwise I'm stumped. :) Yes, the bug name might be confusing. The change is correct. However, it would have been better to document that in |panel.openPopup| but I have no idea if there is such a documentation and the popup isn't at the left in LTR and at the right in RTL but it's inversed in RTL.

I added a sentence to: https://developer.mozilla.org/en/XUL/Method/openPopup to mention that the orientation of the panel varies by locale. I don't think this needs to be more specific...