35.93 KB, image/png
11.54 KB, image/png
858 bytes, patch
|Details | Diff | Splinter Review|
10.04 KB, image/png
See the screenshot.
Probably just need to remove the top and bottom margin from #fullscreen-exit-button in browser/themes/shared/fullscreen/warning.inc.css.
Priority: -- → P3
Whiteboard: [good first bug][lang=css]
I'd like to work on it. Its my first bug, so give me some time.
(In reply to Timothy Pan from comment #2) > I'd like to work on it. Its my first bug, so give me some time. It's not an urgent issue, so take your time! Let me know if you have questions.
Hey, I could not reproduce it in nightly 54.0. And I cannot even see the #fullscreen-exit-button in the specified path.
(In reply to Srivatsav Gunisetty from comment #4) > Hey, I could not reproduce it in nightly 54.0. What operating system are you using? I think this is more visible on macOS than on other platforms. > And I cannot even see the > #fullscreen-exit-button in the specified path. You would need to add a new rule for that.
As far as I can tell it looks like the problem is with the rule "html|*.pointerlockfswarning-exit-button", because it does not to be applied at all. As you suggested once I changed the selector back to #fullscreen-exit-button everything worked, all the declarations including setting margins to 0 were applied. But since all of the other rules work and use a class selector, I suppose the best way to fix this would be to add the class to the button, instead of using id selector.
I Think this issue is not present with nightly(54.0) Linux. I am attaching the screenshot.
(In reply to Timothy Pan from comment #6) > As far as I can tell it looks like the problem is with the rule > "html|*.pointerlockfswarning-exit-button", because it does not to be applied > at all. As you suggested once I changed the selector back to > #fullscreen-exit-button everything worked, all the declarations including > setting margins to 0 were applied. I'm not sure what the point of this obsolete rule is. Looks like nobody missed it except for the margin issue. I'd review a patch that updates the selector and strips down the rule to just removing the margin. > But since all of the other rules work and use a class selector, I suppose > the best way to fix this would be to add the class to the button, instead of > using id selector. This is because those elements occur twice in browser.xul. The button exists only once, though, so let's keep using the id.
hi, can i also work on this bug ? I see Timothy is also trying to work on it.
Timothy, do you want to keep working on this?
Yes, I can do it. I've just been a little busy. I just have to read the documentation on how to submit the fix properly.
ok cool, I will look for another bug!
Comment on attachment 8837214 [details] [diff] [review] Fix CSS rule to select by ID and set margins to 0 only. Looks good. Thanks!
Attachment #8837214 - Flags: review+
Pushed by firstname.lastname@example.org: https://hg.mozilla.org/integration/mozilla-inbound/rev/e3cc6b9b36b3 Change fs warning button css rule to use ID selector and remove margins. r=dao
While verifying the fix of this bug with Latest Nightly 55.0a1 (2017-03-29)on Windows 10 (64 bit), I felt that the "Exit Full Screen" button is not properly aligned. I have attached a screenshot of the fix that I saw.Is the current fix (attached in the attachment) should be expected as the fix? [bugday-20170329]
I can still reproduce this issue on the latest nightly using Windows 10 x64.
You need to log in before you can comment on or make changes to this bug.